[PATCH xorg-gtest 05/16] xserver: move testing startup to the XServer object

[Chase Douglas]

On 07/03/2012 10:34 PM, Peter Hutterer wrote:
> On Tue, Jul 03, 2012 at 10:30:28AM -0700, Chase Douglas wrote:
>> On 07/02/2012 11:44 PM, Peter Hutterer wrote:
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>
>> It makes sense to move the startup to the XServer object, but the
>> environment should then contain an XServer object and start it. The
>> point of the environment is that it starts up an xserver
>> automatically.
>>
>> The xorg-gtest user is free to not use the environment. But when
>> they use the environment, it should start a server.
>
> It still does, it's just cut off from the Environment::SetUp() hunk though.
> see below
>
>>> ---
>>>   include/xorg/gtest/xorg-gtest-xserver.h |    2 ++
>>>   src/environment.cpp                     |   35 ---------------------------
>>>   src/xserver.cpp                         |   40 +++++++++++++++++++++++++++++++
>>>   3 files changed, 42 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
>>> index 5c5ce99..52a2fd0 100644
>>> --- a/include/xorg/gtest/xorg-gtest-xserver.h
>>> +++ b/include/xorg/gtest/xorg-gtest-xserver.h
>>> @@ -129,6 +129,8 @@ class XServer : public xorg::testing::Process {
>>>       XServer(const XServer&);
>>>       XServer& operator=(const XServer&);
>>>
>>> +    void TestStartup(void);
>>> +
>>>   };
>>>   } // namespace testing
>>>   } // namespace xorg
>>> diff --git a/src/environment.cpp b/src/environment.cpp
>>> index 7ed23b3..69972a4 100644
>>> --- a/src/environment.cpp
>>> +++ b/src/environment.cpp
>>> @@ -106,41 +106,6 @@ void xorg::testing::Environment::SetUp() {
>>>     static char display_string[6];
>>>     snprintf(display_string, 6, ":%d", d_->display);
>>>
>>> -  Display* test_display = XOpenDisplay(display_string);
>>> -  if (test_display) {
>>> -    XCloseDisplay(test_display);
>>> -    std::string message;
>>> -    message += "A server is already running on ";
>>> -    message += display_string;
>>> -    message += ".";
>>> -    throw std::runtime_error(message);
>>> -  }
>>> -
>>> -  /* The Xorg server won't start unless the log file and the old log file are
>>> -   * writable. */
>>> -  std::ofstream log_test;
>>> -  log_test.open(d_->path_to_log_file.c_str(), std::ofstream::out);
>>> -  log_test.close();
>>> -  if (log_test.fail()) {
>>> -    std::string message;
>>> -    message += "X.org server log file ";
>>> -    message += d_->path_to_log_file;
>>> -    message += " is not writable.";
>>> -    throw std::runtime_error(message);
>>> -  }
>>> -
>>> -  std::string old_log_file = d_->path_to_log_file.c_str();
>>> -  old_log_file += ".old";
>>> -  log_test.open(old_log_file.c_str(), std::ofstream::out);
>>> -  log_test.close();
>>> -  if (log_test.fail()) {
>>> -    std::string message;
>>> -    message += "X.org old server log file ";
>>> -    message += old_log_file;
>>> -    message += " is not writable.";
>>> -    throw std::runtime_error(message);
>>> -  }
>>> -
>>>     d_->server.SetDisplayNumber(d_->display);
>>>     d_->server.SetLogfilePath(d_->path_to_log_file);
>>>     d_->server.SetConfigPath(d_->path_to_conf);
>
> is followed by
>      d_->server.Start(d_->path_to_server);
>      d_->server.WaitForConnections();
>
> So nothing should change over the previous code. I haven't modified the
> xi2.cpp code in the server which uses this environment and it still works.

Ahh, right, the code still starts the server, but shouldn't we be 
calling the server TestStartup() method before? As it is in this patch, 
we've removed a bunch of code, moved it to a method of a different 
object, and haven't called the method to ensure the same code is 
executed. Unless I'm misreading again :).

>>> diff --git a/src/xserver.cpp b/src/xserver.cpp
>>> index 38394f3..1a46dbb 100644
>>> --- a/src/xserver.cpp
>>> +++ b/src/xserver.cpp
>>> @@ -41,6 +41,7 @@
>>>   #include <cstring>
>>>   #include <stdexcept>
>>>   #include <vector>
>>> +#include <fstream>
>>>
>>>   #include <X11/Xlib.h>
>>>   #include <X11/extensions/XInput2.h>
>>> @@ -250,7 +251,46 @@ void xorg::testing::XServer::WaitForConnections(void) {
>>>     throw std::runtime_error("Unable to open connection to dummy X server");
>>>   }
>>>
>>> +void xorg::testing::XServer::TestStartup(void) {
>>> +  Display* test_display = XOpenDisplay(GetDisplayString());
>>> +  if (test_display) {
>>> +    XCloseDisplay(test_display);
>>> +    std::string message;
>>> +    message += "A server is already running on ";
>>> +    message += GetDisplayString();
>>> +    message += ".";
>>> +    throw std::runtime_error(message);
>>> +  }
>>> +
>>> +  /* The Xorg server won't start unless the log file and the old log file are
>>> +   * writable. */
>>> +  std::ofstream log_test;
>>> +  log_test.open(d_->path_to_logfile.c_str(), std::ofstream::out);
>>> +  log_test.close();
>>> +  if (log_test.fail()) {
>>> +    std::string message;
>>> +    message += "X.org server log file ";
>>> +    message += d_->path_to_logfile;
>>> +    message += " is not writable.";
>>> +    throw std::runtime_error(message);
>>> +  }
>>> +
>>> +  std::string old_log_file = d_->path_to_logfile.c_str();
>>> +  old_log_file += ".old";
>>> +  log_test.open(old_log_file.c_str(), std::ofstream::out);
>>> +  log_test.close();
>>> +  if (log_test.fail()) {
>>> +    std::string message;
>>> +    message += "X.org old server log file ";
>>> +    message += old_log_file;
>>> +    message += " is not writable.";
>>> +    throw std::runtime_error(message);
>>> +  }
>>> +
>>> +}
>>> +
>>>   void xorg::testing::XServer::Start(std::string &program) {
>>> +  TestStartup();
>>>     Process::Start(program, program.c_str(),
>>>                    GetDisplayString(),
>>>                    "-logverbose", "10",
>>>
>>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>