[PATCH v2 xorg-gtest] Added --xorg-logfile option.

[Chase Douglas]

On 01/31/2012 08:57 PM, Daniel d'Andrada wrote:
> On 01/31/2012 05:46 PM, Chase Douglas wrote:
>> On 01/31/2012 06:05 PM, Daniel d'Andrada wrote:
>>> And by default point to a location that doesn't require root privileges
>>> to be used.
>>>
>>> This will make it possible to run Xorg without being root.
>>>
>>> Signed-off-by: Daniel d'Andrada<daniel.dandrada at canonical.com>
>>>
>>> diff:
>>> === modified file 'include/xorg/gtest/environment.h'
>>> --- include/xorg/gtest/environment.h    2011-12-14 19:01:39 +0000
>>> +++ include/xorg/gtest/environment.h    2012-01-31 16:55:22 +0000
>>> @@ -48,6 +48,7 @@
>>>     * with the overall testing framework like
>>>     * @code
>>>     * std::string xorg_conf_path("conf/dummy.conf");
>>> + * std::string xorg_log_file_path("/tmp/MyDummyXorg.log");
>>>     * int xorg_display = 133;
>>>     * std::string server("Xorg");
>>>     *
>>> @@ -55,6 +56,7 @@
>>>     *       xorg_conf_path,
>>>     *       server,
>>>     *       xorg_display);
>>> + * environment->set_log_file(xorg_log_file_path);
>>>     * testing::AddGlobalTestEnvironment(environment);
>>>     * @endcode
>>>     * or link to libxorg-gtest_main.
>>> @@ -72,6 +74,19 @@
>>>
>>>      virtual ~Environment();
>>>
>>> +  /**
>>> +   * Sets the path where the xserver log file will be created.
>>> +   * @param path_to_log_file Path to xserver logfile.
>>> +   */
>>> +  void set_log_file(const std::string&  path_to_log_file);
>>> +
>>> +  /**
>>> +   * Returns the path where the xserver log file will be created.
>>> +   * Its default value is "/tmp/Xorg.GTest.log"
>>> +   * @return Path to xserver logfile.
>>> +   */
>>> +  const std::string&  log_file() const;
>>> +
>>>     protected:
>>>      /**
>>>       * Starts the dummy X server.
>>>
>>> === modified file 'src/environment.cpp'
>>> --- src/environment.cpp    2011-12-14 19:11:25 +0000
>>> +++ src/environment.cpp    2012-01-31 16:55:22 +0000
>>> @@ -34,12 +34,16 @@
>>>
>>>    #include<X11/Xlib.h>
>>>
>>> +#define DEFAULT_XORG_LOGFILE "/tmp/Xorg.GTest.log"
>>> +
>>>    struct xorg::testing::Environment::Private {
>>>      Private(const std::string&  conf, const std::string&  server, int
>>> display_num)
>>> -      : path_to_conf(conf), path_to_server(server), display(display_num) {
>>> +      : path_to_conf(conf), path_to_log_file(DEFAULT_XORG_LOGFILE),
>>> +        path_to_server(server), display(display_num) {
>>>      }
>>>
>>>      const std::string path_to_conf;
>>> +  std::string path_to_log_file;
>>>      const std::string path_to_server;
>>>      const int display;
>>>      Process process;
>>> @@ -53,12 +57,25 @@
>>>
>>>    xorg::testing::Environment::~Environment() {}
>>>
>>> +void xorg::testing::Environment::set_log_file(const std::string&
>>> path_to_log_file)
>>> +{
>>> +  d_->path_to_log_file = path_to_log_file;
>>> +}
>>> +
>>> +const std::string&  xorg::testing::Environment::log_file() const
>>> +{
>>> +  return d_->path_to_log_file;
>>> +}
>>> +
>>>    void xorg::testing::Environment::SetUp() {
>>>      static char display_string[6];
>>>      snprintf(display_string, 6, ":%d", d_->display);
>>>
>>>      d_->process.Start(d_->path_to_server, d_->path_to_server.c_str(),
>>> -                    display_string, "-config",
>>> d_->path_to_conf.c_str(), NULL);
>>> +                    display_string,
>>> +                    "-logfile", d_->path_to_log_file.c_str(),
>>> +                    "-config", d_->path_to_conf.c_str(),
>>> +                    NULL);
>>>
>>>      Process::SetEnv("DISPLAY", display_string, true);
>>>
>>>
>>> === modified file 'src/main.cpp'
>>> --- src/main.cpp    2011-12-07 20:38:04 +0000
>>> +++ src/main.cpp    2012-01-31 16:55:22 +0000
>>> @@ -25,12 +25,15 @@
>>>
>>>    #include "xorg/gtest/environment.h"
>>>
>>> +#define DEFAULT_XORG_LOGFILE "/tmp/Xorg.GTest.log"
>>> +
>>>    namespace {
>>>
>>>    int help = false;
>>>    int no_dummy_server = false;
>>>    int xorg_conf = false;
>>>    int xorg_display_opt = false;
>>> +int xorg_logfile = false;
>>>    int server = false;
>>>
>>>    const struct option longopts[] = {
>>> @@ -38,6 +41,7 @@
>>>      { "no-dummy-server", no_argument,&no_dummy_server, true, },
>>>      { "xorg-conf", required_argument,&xorg_conf, true, },
>>>      { "xorg-display", required_argument,&xorg_display_opt, true, },
>>> +  { "xorg-logfile", required_argument,&xorg_logfile, true, },
>>>      { "server", required_argument,&server, true, },
>>>      { NULL, 0, NULL, 0 }
>>>    };
>>> @@ -48,6 +52,8 @@
>>>      /* Default Xorg dummy conf path. */
>>>      std::string xorg_conf_path(DUMMY_CONF_PATH);
>>>
>>> +  std::string xorg_log_file_path(DEFAULT_XORG_LOGFILE);
>>> +
>>>      /* Default X display */
>>>      int xorg_display = 133;
>>>
>>> @@ -80,6 +86,10 @@
>>>            break;
>>>
>>>          case 4:
>>> +        xorg_log_file_path = optarg;
>>> +        break;
>>> +
>>> +      case 5:
>>>            server = optarg;
>>>            break;
>>>
>>> @@ -95,6 +105,8 @@
>>>        std::cout<<  "    --xorg-conf: Path to xorg dummy configuration
>>> file\n";
>>>        std::cout<<  "    --server: Path to X server executable\n";
>>>        std::cout<<  "    --xorg-display: xorg dummy display port\n";
>>> +    std::cout<<  "    --xorg-logfile: xorg logfile filename. See
>>> -logfile in \"man Xorg\".\n"
>>> +                 "                    Its default value is
>>> "DEFAULT_XORG_LOGFILE".\n";
>>>        exit(-1);
>>>      }
>>>
>>> @@ -103,6 +115,7 @@
>>>            xorg_conf_path,
>>>            server,
>>>            xorg_display);
>>> +    environment->set_log_file(xorg_log_file_path);
>>>        testing::AddGlobalTestEnvironment(environment);
>>>      }
>> If the log file isn't specified, I think we should leave it up to the
>> library to put it in the right location.
>>
>> -- Chase
> 
> Isn't that what's happening? If the log file isn't specified it will be 
> /tmp/Xorg.GTest.log.
> Or what you mean is that you don't want to see 
> environment->set_log_file() being always called from main.cpp?

Yeah, the latter. It should be:

if (xorg_logfile)
  environment->set_log_file(xorg_log_file_path);