On 07/11/2012 04:07 PM, Peter Hutterer wrote:
On Wed, Jul 11, 2012 at 11:57:50AM -0700, Chase Douglas wrote:
On 07/10/2012 08:28 PM, Peter Hutterer wrote:
Changes from hardcoded vararg call to requiring the caller to set up options
for logfile, config, etc. beforehand. The display string is automatically
added to the command line.
If no binary is given by the caller, the previously set binary path is used.
Signed-off-by: Peter Hutterer <[email protected]>
---
Changes to v1:
- have one Start() call only handling with/without binary
- changes to accommodate for Process::Start(vector<std::string>) instead of
previous const char *
include/xorg/gtest/xorg-gtest-xserver.h | 8 ++++++++
src/environment.cpp | 7 +------
src/xserver.cpp | 19 +++++++++++++++++++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/include/xorg/gtest/xorg-gtest-xserver.h
b/include/xorg/gtest/xorg-gtest-xserver.h
index 1ba2a0f..0b7e7a0 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -47,6 +47,14 @@ class XServer : public xorg::testing::Process {
XServer();
/**
+ * Start a new server. If no binary is given, the server started is the
+ * default compiled-in server binary.
+ *
+ * @param [in] program Path to the XServer binary
+ */
+ void Start(std::string program = std::string());
I'm still not sure we need the program argument. I would rather we
stick to either setting the program path with SetServerPath(), or
providing it in Start() as you have here. My preference would be to
just drop SetServerPath(), but either approach is fine with me.
I think both will be useful, for different types of tests. SetServerPath is
good for tests where we start up the same (non-standard) server multiple
times:
setup():
XServer s;
s.SetServerPath("...");
test():
s.Start();
// test bits
s.Terminate()
s.Start();
// test bits
s.Terminate();
...
the program argument is useful for testing other DDXs:
s.Start("Xorg");
// test bits
s.Terminate()
s.Start("Xephyr");
// test bits
s.Terminate();
....
of course, none of them is impossible with the other approach, and I don't
actually have test cases for the second bit, though they're not far out.
it's just a convenience API.
I wondered if this wasn't the case :). Ok, sounds alright to me.
If you do leave this, pass in a const reference rather than a copy
of the string. I see that you reuse the variable in the method, but
using that as a reason to copy the string in would be leaking the
implementation into the API.
done.
+
+ /**
* Waits until this server is ready to take connections.
*/
void WaitForConnections(void);
diff --git a/src/environment.cpp b/src/environment.cpp
index 7573b62..1521564 100644
--- a/src/environment.cpp
+++ b/src/environment.cpp
@@ -144,12 +144,7 @@ void xorg::testing::Environment::SetUp() {
d_->server.SetDisplayNumber(d_->display);
d_->server.SetOption("-logfile", d_->path_to_log_file);
d_->server.SetOption("-config", d_->path_to_conf);
- d_->server.Start(d_->path_to_server, d_->path_to_server.c_str(),
- display_string,
- "-logverbose", "10",
- "-logfile", d_->path_to_log_file.c_str(),
- "-config", d_->path_to_conf.c_str(),
- NULL);
+ d_->server.Start(d_->path_to_server);
Since you use d_->path_to_server as the default in Start(), there's
no need to pass it in here.
the environment's path_to_server may differ to the built-in default from the
server, so we do need to pass this here.
Ok, I got it.
-- Chase
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
