[PATCH xorg-gtest] Make Environment API property-based.

[Chase Douglas]

On 02/06/2012 12:29 PM, Daniel d'Andrada wrote:
> Instead of shoving all parameters in the constructor.
> 
> Signed-off-by: Daniel d'Andrada <daniel.dandrada at canonical.com>
> ---
>  include/xorg/gtest/environment.h |   64 +++++++++++++++++++++++++++++---------
>  src/Makefile.am                  |    6 ++--
>  src/defines.h                    |    2 +
>  src/environment.cpp              |   48 ++++++++++++++++++++++------
>  src/main.cpp                     |   37 +++++++++++----------
>  5 files changed, 111 insertions(+), 46 deletions(-)
> 
> diff --git a/include/xorg/gtest/environment.h b/include/xorg/gtest/environment.h
> index 3996507..22c82be 100644
> --- a/include/xorg/gtest/environment.h
> +++ b/include/xorg/gtest/environment.h
> @@ -47,16 +47,11 @@ namespace testing {
>   * Either associate the environment manually
>   * 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");
> - *
> - * xorg::testing::Environment* environment = new xorg::testing::Environment(
> - *       xorg_conf_path,
> - *       server,
> - *       xorg_display);
> - * environment->set_log_file(xorg_log_file_path);
> + * xorg::testing::Environment* environment = new xorg::testing::Environment;
> + * environment->set_server("Xorg");
> + * environment->set_display(133);
> + * environment->set_conf_file("conf/dummy.conf");
> + * environment->set_log_file("/tmp/MyDummyXorg.log");
>   * testing::AddGlobalTestEnvironment(environment);
>   * @endcode
>   * or link to libxorg-gtest_main.
> @@ -65,12 +60,8 @@ class Environment : public ::testing::Environment {
>   public:
>    /**
>     * Constructs an object to provide a global X server dummy environment.
> -   * @param path_to_conf Path to xserver configuration.
> -   * @param path_to_server Path to xserver executable.
> -   * @param display Display port of dummy xserver instance.
>     */
> -  Environment(const std::string& path_to_conf,
> -              const std::string& path_to_server = "Xorg", int display = 133);
> +  Environment();
>  
>    virtual ~Environment();
>  
> @@ -87,6 +78,49 @@ class Environment : public ::testing::Environment {
>     */
>    const std::string& log_file() const;
>  
> +  /**
> +   * Sets the path to the desired xserver configuration file.
> +   *
> +   * It will be passed on to the xserver via the command line
> +   * argument "-config".
> +   *
> +   * @param path_conf_file Path to a Xorg X server .conf file.
> +   */
> +  void set_conf_file(const std::string& path_conf_file);
> +
> +  /**
> +   * Returns the path of the xserver configuration file to be used.
> +   * Its default value is [datadir]/xorg/gtest/dummy.conf
> +   * @return File path of the xserver configuration currently set
> +   */
> +  const std::string& conf_file() const;
> +
> +  /**
> +   * Sets the path to the xserver executable
> +   * @param path_to_server Path to a xserver executable
> +   */
> +  void set_server(const std::string& path_to_server);
> +
> +  /**
> +   * Returns the path where the xserver executable to be used.
> +   * Its default value is "Xorg"
> +   * @return Path to xserver executable.
> +   */
> +  const std::string& server() const;
> +
> +  /**
> +   * Sets the display number that the xserver will use.
> +   * @param diplay_num A display number.
> +   */
> +  void set_display(int display_num);
> +
> +  /**
> +   * Returns the display number of the xserver instance.
> +   * Its default value is 133
> +   * @return Display number of the xserver.
> +   */
> +  int display() const;
> +

I prefer the format:

/**
 * Short description
 *
 * Long description and/or details
 *
 * @param lines, etc.
 */

The line separating the short and long description is required due to
our Doxygen setup.

I ammended the commit to put the comments in this format. I also moved
the default value comments to the setter functions. I think that's where
most people will look to find default values.

Everything else looks good! I pushed the patch to master.

Thanks!

-- Chase