You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Santhosh Edukulla <sa...@citrix.com> on 2013/10/29 13:33:48 UTC
Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/
-----------------------------------------------------------
Review request for cloudstack and Prasanna Santhanam.
Repository: cloudstack-git
Description
-------
Added Configuration Support to Marvin.
1. It provides the basic configuration facilities to marvin.
2. User can just add configuration files for his tests, deployment
etc, under one config folder before running their tests.
cs/tools/marvin/marvin/config.
They can remove all hard coded values from code and separate
it out as config at this location.
Either add this to the existing setup.cfg as separate section
or add new configuration.
3. This will thus removes hard coded tests and separate
data from tests.
4. This API is provided as an additional facility under
cloudstackTestClient and users can get the
configuration object as similar to apiclient,dbconnection
etc to drive their test.
5. They just add their configuration for a test,
setup etc,at one single place under configuration dir
and use "getConfigParser" API of cloudstackTestClient
It will give them "configObj".They can either pass their own
config file for parsing to "getConfig" or it will use
default config file @ config/setup.cfg.
6. They will then get the dictionary of parsed
configuration and can use it further to drive their tests or
config drive
7. Test features, can drive their setups thus removing hard coded
values. Configuration default file will be under config and as
setup.cfg.
8. Users can use their own configuration file passed to
"getConfig" API,once configObj is returned.
Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
Diffs
-----
tools/marvin/marvin/cloudstackTestClient.py be93f35
tools/marvin/marvin/config/setup.cfg PRE-CREATION
tools/marvin/marvin/configGenerator.py 0cfad30
tools/marvin/marvin/integration/lib/utils.py 7d662af
Diff: https://reviews.apache.org/r/15021/diff/
Testing
-------
Basic check to retrieve configuration values is done.
Thanks,
Santhosh Edukulla
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27887
-----------------------------------------------------------
Ship it!
0b617a1
- Prasanna Santhanam
On Oct. 31, 2013, 11:57 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 11:57 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> File Attachments
> ----------------
>
> Added changes as discussed
> https://reviews.apache.org/media/uploaded/files/2013/10/31/0001-Fixed-Bug-4899_2.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Girish Shilamkar <gi...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review28512
-----------------------------------------------------------
4.2: e7b6ee10eda85a90c594c9de5d36cc062e069576
- Girish Shilamkar
On Oct. 31, 2013, 11:57 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 11:57 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> File Attachments
> ----------------
>
> Added changes as discussed
> https://reviews.apache.org/media/uploaded/files/2013/10/31/0001-Fixed-Bug-4899_2.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 11:57 a.m.)
Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
Changes
-------
1. Removed few unwanted imports
2. Added default config dir path
3. Added config directory to setup.py
4. Added config file path to be pickedup dynamically.
Repository: cloudstack-git
Description
-------
Added Configuration Support to Marvin.
1. It provides the basic configuration facilities to marvin.
2. User can just add configuration files for his tests, deployment
etc, under one config folder before running their tests.
cs/tools/marvin/marvin/config.
They can remove all hard coded values from code and separate
it out as config at this location.
Either add this to the existing setup.cfg as separate section
or add new configuration.
3. This will thus removes hard coded tests and separate
data from tests.
4. This API is provided as an additional facility under
cloudstackTestClient and users can get the
configuration object as similar to apiclient,dbconnection
etc to drive their test.
5. They just add their configuration for a test,
setup etc,at one single place under configuration dir
and use "getConfigParser" API of cloudstackTestClient
It will give them "configObj".They can either pass their own
config file for parsing to "getConfig" or it will use
default config file @ config/setup.cfg.
6. They will then get the dictionary of parsed
configuration and can use it further to drive their tests or
config drive
7. Test features, can drive their setups thus removing hard coded
values. Configuration default file will be under config and as
setup.cfg.
8. Users can use their own configuration file passed to
"getConfig" API,once configObj is returned.
Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
ToDo:
clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
Diffs
-----
tools/marvin/marvin/cloudstackTestClient.py be93f35
tools/marvin/marvin/config/setup.cfg PRE-CREATION
tools/marvin/marvin/configGenerator.py 0cfad30
tools/marvin/marvin/integration/lib/utils.py 7d662af
Diff: https://reviews.apache.org/r/15021/diff/
Testing
-------
Basic check to retrieve configuration values is done.
File Attachments (updated)
----------------
Added changes as discussed
https://reviews.apache.org/media/uploaded/files/2013/10/31/0001-Fixed-Bug-4899_2.patch
Thanks,
Santhosh Edukulla
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Oct. 30, 2013, 10:22 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/cloudstackTestClient.py, line 22
> > <https://reviews.apache.org/r/15021/diff/1/?file=373112#file373112line22>
> >
> > It came from merge, i didnt used them in modules required.
This i will change to remove unused imports in a different patch.
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27765
-----------------------------------------------------------
On Oct. 30, 2013, 10:12 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 10:12 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Oct. 30, 2013, 10:22 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/configGenerator.py, line 337
> > <https://reviews.apache.org/r/15021/diff/1/?file=373114#file373114line337>
> >
> > Nopes, its right, user by default no need to mention any path while creating the object instead provides it under getConfig. User by default dont need to worry about the configuration provided by default, we are providing him the parsed dictionary by default by placing the configuration at a default location. Only if he wanted to change this configuration to a separate one he can still change by using getConfig method.
>
> Prasanna Santhanam wrote:
> so self.__init__(self, filepath='defaultpath'): should work. The reason for adding the filepath argument is simply to aid call to the command line switch from nose. By losing the command line switch from nose to specify the marvin-config path, do you not feel we are restricting?
>
> also, i guess i'm having a hard time seeing how the said user will use this getConfig utility. can you perhaps re-use the ldap test to show how the hardcoded external resource is simplified using ConfigManager? that would make it easier to understand and this patch can be absorbed with that test.
>
Added a simple usage of how this facility can be used as an example.
'''
1. cloudstackTestClient will now have configuration object exposed
and available through its method getConfigParser()
2. The object will now have all options exposed through ConfigManager
3. cls.configObj will now provide ConfigManager interface
User can use this to parse and get the dictionary represenation
of his configuaration
'''
@classmethod
def setUpClass(cls):
cls.api_client = super(
TestAddMultipleNetScaler,
cls
).getClsTestClient().getApiClient()
cls.configObj = super(
TestAddMultipleNetScaler,
cls
).getConfigParser()
if cls.configObj is not None:
'''
Here, cls.configDict will now have all the configuration parsed and available as Dictionary
This Dictionary is available to total test feature code accessible wherever applicable
'''
cls.configDict = cls.configObj.getConfig(self, "/setup/config/myfeature.cfg", None )
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27765
-----------------------------------------------------------
On Oct. 30, 2013, 10:12 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 10:12 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
> On Oct. 30, 2013, 10:22 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/configGenerator.py, line 303
> > <https://reviews.apache.org/r/15021/diff/1/?file=373114#file373114line303>
> >
> > The reason is simple. We already have a module configGenerator and is used as configuration utility ( reading configuraiton, object mapping and various few things ). I dont want to create multiple modules with config** name, it adds some confusion to users to understand and unnecesarily module creation. Ideally it can be separate, but the task is achieved through a simple class inside of a configGenerator
since this module is providing facilities to tests it would've been nice to move it out. but i'm not strongly against your way either.
> On Oct. 30, 2013, 10:22 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/configGenerator.py, line 337
> > <https://reviews.apache.org/r/15021/diff/1/?file=373114#file373114line337>
> >
> > Nopes, its right, user by default no need to mention any path while creating the object instead provides it under getConfig. User by default dont need to worry about the configuration provided by default, we are providing him the parsed dictionary by default by placing the configuration at a default location. Only if he wanted to change this configuration to a separate one he can still change by using getConfig method.
so self.__init__(self, filepath='defaultpath'): should work. The reason for adding the filepath argument is simply to aid call to the command line switch from nose. By losing the command line switch from nose to specify the marvin-config path, do you not feel we are restricting?
also, i guess i'm having a hard time seeing how the said user will use this getConfig utility. can you perhaps re-use the ldap test to show how the hardcoded external resource is simplified using ConfigManager? that would make it easier to understand and this patch can be absorbed with that test.
> On Oct. 30, 2013, 10:22 a.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/cloudstackTestClient.py, line 22
> > <https://reviews.apache.org/r/15021/diff/1/?file=373112#file373112line22>
> >
> > It came from merge, i didnt used them in modules required.
>
> Santhosh Edukulla wrote:
> This i will change to remove unused imports in a different patch.
can you readjust this patch and rebase atop master?
- Prasanna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27765
-----------------------------------------------------------
On Oct. 30, 2013, 10:12 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 10:12 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27765
-----------------------------------------------------------
tools/marvin/marvin/cloudstackTestClient.py
<https://reviews.apache.org/r/15021/#comment53957>
It came from merge, i didnt used them in modules required.
tools/marvin/marvin/configGenerator.py
<https://reviews.apache.org/r/15021/#comment53958>
The reason is simple. We already have a module configGenerator and is used as configuration utility ( reading configuraiton, object mapping and various few things ). I dont want to create multiple modules with config** name, it adds some confusion to users to understand and unnecesarily module creation. Ideally it can be separate, but the task is achieved through a simple class inside of a configGenerator
tools/marvin/marvin/configGenerator.py
<https://reviews.apache.org/r/15021/#comment53959>
Nopes, its right, user by default no need to mention any path while creating the object instead provides it under getConfig. User by default dont need to worry about the configuration provided by default, we are providing him the parsed dictionary by default by placing the configuration at a default location. Only if he wanted to change this configuration to a separate one he can still change by using getConfig method.
- Santhosh Edukulla
On Oct. 30, 2013, 10:12 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 10:12 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/
-----------------------------------------------------------
(Updated Oct. 30, 2013, 10:12 a.m.)
Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
Repository: cloudstack-git
Description
-------
Added Configuration Support to Marvin.
1. It provides the basic configuration facilities to marvin.
2. User can just add configuration files for his tests, deployment
etc, under one config folder before running their tests.
cs/tools/marvin/marvin/config.
They can remove all hard coded values from code and separate
it out as config at this location.
Either add this to the existing setup.cfg as separate section
or add new configuration.
3. This will thus removes hard coded tests and separate
data from tests.
4. This API is provided as an additional facility under
cloudstackTestClient and users can get the
configuration object as similar to apiclient,dbconnection
etc to drive their test.
5. They just add their configuration for a test,
setup etc,at one single place under configuration dir
and use "getConfigParser" API of cloudstackTestClient
It will give them "configObj".They can either pass their own
config file for parsing to "getConfig" or it will use
default config file @ config/setup.cfg.
6. They will then get the dictionary of parsed
configuration and can use it further to drive their tests or
config drive
7. Test features, can drive their setups thus removing hard coded
values. Configuration default file will be under config and as
setup.cfg.
8. Users can use their own configuration file passed to
"getConfig" API,once configObj is returned.
Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
ToDo:
clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
Diffs
-----
tools/marvin/marvin/cloudstackTestClient.py be93f35
tools/marvin/marvin/config/setup.cfg PRE-CREATION
tools/marvin/marvin/configGenerator.py 0cfad30
tools/marvin/marvin/integration/lib/utils.py 7d662af
Diff: https://reviews.apache.org/r/15021/diff/
Testing
-------
Basic check to retrieve configuration values is done.
Thanks,
Santhosh Edukulla
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Oct. 30, 2013, 10:12 a.m., Prasanna Santhanam wrote:
> > tools/marvin/marvin/configGenerator.py, line 338
> > <https://reviews.apache.org/r/15021/diff/1/?file=373114#file373114line338>
> >
> > Can we have this driven by a cmd line switch to nose?
Yes,the facility is now added, the current marvin-config switch will be used as default configuration replacing the current config/setup.cfg. This facility is part of TODO things as part of cleanup where we do clean up for the existing configuration handling.
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27764
-----------------------------------------------------------
On Oct. 30, 2013, 10:12 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 10:12 a.m.)
>
>
> Review request for cloudstack, Girish Shilamkar and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/#review27764
-----------------------------------------------------------
tools/marvin/marvin/cloudstackTestClient.py
<https://reviews.apache.org/r/15021/#comment53956>
these imports are unused in the module - random, string, hashlib. please remove them.
tools/marvin/marvin/configGenerator.py
<https://reviews.apache.org/r/15021/#comment53953>
perhaps the configGenerator and the configManager can be separate modules? Is there a reason this should be included in the codegenerator.py module?
tools/marvin/marvin/configGenerator.py
<https://reviews.apache.org/r/15021/#comment53955>
The signature should perhaps change to __init__(self, path) where path is the config file?
tools/marvin/marvin/configGenerator.py
<https://reviews.apache.org/r/15021/#comment53954>
Can we have this driven by a cmd line switch to nose?
- Prasanna Santhanam
On Oct. 29, 2013, 6 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15021/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2013, 6 p.m.)
>
>
> Review request for cloudstack and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
> They can remove all hard coded values from code and separate
> it out as config at this location.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration
Support to Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
On Wed, Oct 30, 2013 at 10:42:48AM +0000, Santhosh Edukulla wrote: >
> ConfigManager is as synonymous to any other configuration facility
> provided for products. Currently, we provide configuration as part
> of some file say json\cfg and provide the command line option to use
> that. Now, there are places where the hard coding specific to
> setup\data is done inside of test data. I have given one simple
> example related to ldap search and replace. In order to replace
> these issues, we can use the ConfigManager facility added. We can
> now extend it to some more usecases if required. It has now some
> provision to read the config, abstract to users and obtain to test
> features as easy to use dictionary.
Sure - I understand the intent. This cleanup is good. Can you show how the ldap
test changes with the introduction of ConfigManager? I guess that will make it
clear how to use this. It should also serve as a test for your changes.
>
> The new facility added handles all that mentioned in doc strings. It
> has a simple interface to retrieve the configuration provided and
> users can use it easily inside the test features as a dictionary.
> With this, all configuration pertaining to run will be at one single
> place and we provide a simple usage of this through dictionary. It
> doesn't have any setters for that matter. With this, all hard coding
> will be moved to configuration and users use the dynamic dictionary
> created based upon config and is more driven through config. We can
> remove current not a cleaner way of search and replace with this.
I guess I don't understand how the config that we inject into the test right now
is any different from the ConfigManager interface proposed here. Example, to
reference the pod information of a deployment for example one would do
config.zones[0].pods[0] now. This config attribute is part of the test already.
I'm sure this can be more intuitive to access using a dictionary approach. But
the structure and depth of the keys of that dictionary is going to be in the
hands of the user?
>
> Users mention all
>parameters related to their test in config file, and they have a dictionary of
>the parsed config inside of their tests to use this data. The user don't need
>to hard code this. Of a test as we are doing currently. On a given setup they
>can use a different config and it works to work with those ips set up
>infrastructure dynamically. If I have a ldap server ip different from other,
>the code still works, we just need to change the config.
And we already do this: the deployment config within Citrix labs is vastly
different from the one used on jenkins.bacd.org. So how is this more automatic
when using the configmanager? One would still need to go change the json config
manually, correct? Sounds like we are moving external elements into the config
file from the test? which is how it was always intended. May be we just have a
poorly written test?
>
> The features, where this hard coding is done will now be replaced with this
> usage of moving those hard coded strings to configuration file and replace
> those with config dictionary values, which will be filled in dynamically.
>
> What else you see is missing? Let me know and I can enhance it to add.
>
So the ldap example if you could change using the configManager would be useful
to see how the change would be helpful.
>
> Santhosh
>
> -----Original Message-----
> From: Prasanna Santhanam [mailto:tsp@apache.org]
> Sent: Wednesday, October 30, 2013 3:42 PM
> To: Santhosh Edukulla
> Cc: Prasanna Santhanam; cloudstack
> Subject: Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to Marvin.
>
> I've given some inline comments in the patch. The ConfigManager looks too simple perhaps because it's a WIP? At least the goals as you've written in your docstring seem to talk about more than what's provided in the module.
>
> On Tue, Oct 29, 2013 at 06:00:14PM -0000, Santhosh Edukulla wrote:
> > -------
> >
> > Added Configuration Support to Marvin.
> >
> > 1. It provides the basic configuration facilities to marvin.
> >
> How will this be different from the current configGenerator module?
> The configGenerator could read and write a structured deployment configuration. Is the goal of this configGenerator different?
>
> > 2. User can just add configuration files for his tests, deployment
> > etc, under one config folder before running their tests.
> > cs/tools/marvin/marvin/config.
>
> This seems restrictive because previously one could have their configs placed anywhere. what would be the advantage of placing all configs in tools/marvin/marvin/config? When packaged and deployed this path is going to change to /usr/local/lib/python/site-packages/marvin/config.
> Do we want that?
>
> > They can remove all hard coded values from code and separate
> > it out as config at this location.
>
> How would the ConfigParser know where to find a specific section and at what depth? Is there a structure? Or do you leave that up to the test author? I'd rather not do the latter as things go wild with too many different ways to do the same thing.
>
> > Either add this to the existing setup.cfg as separate section
> > or add new configuration.
> > 3. This will thus removes hard coded tests and separate
> > data from tests.
>
> Really? I think a lot of data is not specific to deployment configuration today in tests. If we unify all data into the deployment json it's going to become a mess to manage, edit and control when running tests.
>
> >
> > 4. This API is provided as an additional facility under
> > cloudstackTestClient and users can get the
> > configuration object as similar to apiclient,dbconnection
> > etc to drive their test.
>
> marvinPlugin.py,L#158 - the config is already injected into each test.
> I like it that it's part of the testClient, wil you be providing setter properties to it if it's part of the testClient via ConfigManager. Test case authors might be tempted to change configuration on the fly which might affect downstream tests in the same suite?
>
> >
> > 5. They just add their configuration for a test,
> > setup etc,at one single place under configuration dir
> > and use "getConfigParser" API of cloudstackTestClient
> > It will give them "configObj".They can either pass their own
> > config file for parsing to "getConfig" or it will use
> > default config file @ config/setup.cfg.
> > 6. They will then get the dictionary of parsed
> > configuration and can use it further to drive their tests or
> > config drive
> > 7. Test features, can drive their setups thus removing hard coded
> > values. Configuration default file will be under config and as
> > setup.cfg.
> > 8. Users can use their own configuration file passed to
> > "getConfig" API,once configObj is returned.
> >
> > Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
> >
> > Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
> >
> > ToDo:
> > clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
> >
> >
> > Diffs
> > -----
> >
> > tools/marvin/marvin/cloudstackTestClient.py be93f35
> > tools/marvin/marvin/config/setup.cfg PRE-CREATION
> > tools/marvin/marvin/configGenerator.py 0cfad30
> > tools/marvin/marvin/integration/lib/utils.py 7d662af
> >
> > Diff: https://reviews.apache.org/r/15021/diff/
> >
> >
> > Testing
> > -------
> >
> > Basic check to retrieve configuration values is done.
> >
> >
> > Thanks,
> >
> > Santhosh Edukulla
> >
>
> --
> Prasanna.,
>
> ------------------------
> Powered by BigRock.com
--
Prasanna.,
------------------------
Powered by BigRock.com
RE: Review Request 15021: Fixed Bug: 4899 : Added Configuration
Support to Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
ConfigManager is as synonymous to any other configuration facility provided for products. Currently, we provide configuration as part of some file say json\cfg and provide the command line option to use that. Now, there are places where the hard coding specific to setup\data is done inside of test data. I have given one simple example related to ldap search and replace. In order to replace these issues, we can use the ConfigManager facility added. We can now extend it to some more usecases if required. It has now some provision to read the config, abstract to users and obtain to test features as easy to use dictionary.
The new facility added handles all that mentioned in doc strings. It has a simple interface to retrieve the configuration provided and users can use it easily inside the test features as a dictionary. With this, all configuration pertaining to run will be at one single place and we provide a simple usage of this through dictionary. It doesn't have any setters for that matter. With this, all hard coding will be moved to configuration and users use the dynamic dictionary created based upon config and is more driven through config. We can remove current not a cleaner way of search and replace with this.
Currently, testclient provides apiclient,dbconnection and added to it will provide configuration interface as well. Using this, test features will get configuration facility for his feature for use.
Users mention all parameters related to their test in config file, and they have a dictionary of the parsed config inside of their tests to use this data. The user don't need to hard code this. Of a test as we are doing currently. On a given setup they can use a different config and it works to work with those ips set up infrastructure dynamically. If I have a ldap server ip different from other, the code still works, we just need to change the config.
The features, where this hard coding is done will now be replaced with this usage of moving those hard coded strings to configuration file and replace those with config dictionary values, which will be filled in dynamically.
What else you see is missing? Let me know and I can enhance it to add.
Santhosh
-----Original Message-----
From: Prasanna Santhanam [mailto:tsp@apache.org]
Sent: Wednesday, October 30, 2013 3:42 PM
To: Santhosh Edukulla
Cc: Prasanna Santhanam; cloudstack
Subject: Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to Marvin.
I've given some inline comments in the patch. The ConfigManager looks too simple perhaps because it's a WIP? At least the goals as you've written in your docstring seem to talk about more than what's provided in the module.
On Tue, Oct 29, 2013 at 06:00:14PM -0000, Santhosh Edukulla wrote:
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
How will this be different from the current configGenerator module?
The configGenerator could read and write a structured deployment configuration. Is the goal of this configGenerator different?
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
This seems restrictive because previously one could have their configs placed anywhere. what would be the advantage of placing all configs in tools/marvin/marvin/config? When packaged and deployed this path is going to change to /usr/local/lib/python/site-packages/marvin/config.
Do we want that?
> They can remove all hard coded values from code and separate
> it out as config at this location.
How would the ConfigParser know where to find a specific section and at what depth? Is there a structure? Or do you leave that up to the test author? I'd rather not do the latter as things go wild with too many different ways to do the same thing.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
Really? I think a lot of data is not specific to deployment configuration today in tests. If we unify all data into the deployment json it's going to become a mess to manage, edit and control when running tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
marvinPlugin.py,L#158 - the config is already injected into each test.
I like it that it's part of the testClient, wil you be providing setter properties to it if it's part of the testClient via ConfigManager. Test case authors might be tempted to change configuration on the fly which might affect downstream tests in the same suite?
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
--
Prasanna.,
------------------------
Powered by BigRock.com
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration
Support to Marvin.
Posted by Prasanna Santhanam <ts...@apache.org>.
I've given some inline comments in the patch. The ConfigManager looks
too simple perhaps because it's a WIP? At least the goals as you've
written in your docstring seem to talk about more than what's
provided in the module.
On Tue, Oct 29, 2013 at 06:00:14PM -0000, Santhosh Edukulla wrote:
> -------
>
> Added Configuration Support to Marvin.
>
> 1. It provides the basic configuration facilities to marvin.
>
How will this be different from the current configGenerator module?
The configGenerator could read and write a structured deployment
configuration. Is the goal of this configGenerator different?
> 2. User can just add configuration files for his tests, deployment
> etc, under one config folder before running their tests.
> cs/tools/marvin/marvin/config.
This seems restrictive because previously one could have their configs
placed anywhere. what would be the advantage of placing all configs in
tools/marvin/marvin/config? When packaged and deployed this path is
going to change to /usr/local/lib/python/site-packages/marvin/config.
Do we want that?
> They can remove all hard coded values from code and separate
> it out as config at this location.
How would the ConfigParser know where to find a specific section and
at what depth? Is there a structure? Or do you leave that up to the
test author? I'd rather not do the latter as things go wild with too
many different ways to do the same thing.
> Either add this to the existing setup.cfg as separate section
> or add new configuration.
> 3. This will thus removes hard coded tests and separate
> data from tests.
Really? I think a lot of data is not specific to deployment
configuration today in tests. If we unify all data into the deployment
json it's going to become a mess to manage, edit and control when
running tests.
>
> 4. This API is provided as an additional facility under
> cloudstackTestClient and users can get the
> configuration object as similar to apiclient,dbconnection
> etc to drive their test.
marvinPlugin.py,L#158 - the config is already injected into each test.
I like it that it's part of the testClient, wil you be providing
setter properties to it if it's part of the testClient via
ConfigManager. Test case authors might be tempted to change
configuration on the fly which might affect downstream tests in the
same suite?
>
> 5. They just add their configuration for a test,
> setup etc,at one single place under configuration dir
> and use "getConfigParser" API of cloudstackTestClient
> It will give them "configObj".They can either pass their own
> config file for parsing to "getConfig" or it will use
> default config file @ config/setup.cfg.
> 6. They will then get the dictionary of parsed
> configuration and can use it further to drive their tests or
> config drive
> 7. Test features, can drive their setups thus removing hard coded
> values. Configuration default file will be under config and as
> setup.cfg.
> 8. Users can use their own configuration file passed to
> "getConfig" API,once configObj is returned.
>
> Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
>
> Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
>
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/cloudstackTestClient.py be93f35
> tools/marvin/marvin/config/setup.cfg PRE-CREATION
> tools/marvin/marvin/configGenerator.py 0cfad30
> tools/marvin/marvin/integration/lib/utils.py 7d662af
>
> Diff: https://reviews.apache.org/r/15021/diff/
>
>
> Testing
> -------
>
> Basic check to retrieve configuration values is done.
>
>
> Thanks,
>
> Santhosh Edukulla
>
--
Prasanna.,
------------------------
Powered by BigRock.com
Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to
Marvin.
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15021/
-----------------------------------------------------------
(Updated Oct. 29, 2013, 6 p.m.)
Review request for cloudstack and Prasanna Santhanam.
Repository: cloudstack-git
Description (updated)
-------
Added Configuration Support to Marvin.
1. It provides the basic configuration facilities to marvin.
2. User can just add configuration files for his tests, deployment
etc, under one config folder before running their tests.
cs/tools/marvin/marvin/config.
They can remove all hard coded values from code and separate
it out as config at this location.
Either add this to the existing setup.cfg as separate section
or add new configuration.
3. This will thus removes hard coded tests and separate
data from tests.
4. This API is provided as an additional facility under
cloudstackTestClient and users can get the
configuration object as similar to apiclient,dbconnection
etc to drive their test.
5. They just add their configuration for a test,
setup etc,at one single place under configuration dir
and use "getConfigParser" API of cloudstackTestClient
It will give them "configObj".They can either pass their own
config file for parsing to "getConfig" or it will use
default config file @ config/setup.cfg.
6. They will then get the dictionary of parsed
configuration and can use it further to drive their tests or
config drive
7. Test features, can drive their setups thus removing hard coded
values. Configuration default file will be under config and as
setup.cfg.
8. Users can use their own configuration file passed to
"getConfig" API,once configObj is returned.
Another such case where we are using sed or bash script is in between a build run for replacing hard coded ldap ip for region\setup specific. We can now change all parameters before run under configuration, the test features will use configuration object and thus values, rather hard coded strings which avoids replacement through shell script.
Also, did few naming convention changes. Its better to follow uniform naming conventions. Currently, wherever iam seeing a specific notation not followed. We are incorporating those changes.
ToDo:
clean up of current config at places, making configuration required for marvin\tests unified and available at one place and clean up of files\code related to it.
Diffs
-----
tools/marvin/marvin/cloudstackTestClient.py be93f35
tools/marvin/marvin/config/setup.cfg PRE-CREATION
tools/marvin/marvin/configGenerator.py 0cfad30
tools/marvin/marvin/integration/lib/utils.py 7d662af
Diff: https://reviews.apache.org/r/15021/diff/
Testing
-------
Basic check to retrieve configuration values is done.
Thanks,
Santhosh Edukulla