[PATCH xorg-gtest 05/16] xserver: move testing startup to the XServer object
Chase Douglas
chase.douglas at canonical.com
Fri Jul 6 10:59:11 PDT 2012
On 07/05/2012 04:27 PM, Peter Hutterer wrote:
> On Thu, Jul 05, 2012 at 01:46:42PM -0700, Chase Douglas wrote:
>> 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 :).
>
> hard to see from the patch I guess, but the new sequence is
> Environment::SetUp()
> ...
> XServer::Start()
> XServer::TestStartup()
> Process::Start()
Ok, that looks good. I guess some of the context is just getting strewn
across a couple different patches.
-- Chase
More information about the xorg-devel
mailing list