Design for a supported way to change default setup in PTL

Developers
21

I find this feature interesting, but I am somewhat confused about its purpose. If we use the current configuration instead of OOB to run tests, we’re basically doing a hybrid manual/automated test. The purpose of PTL is to write automated tests, do we want this hybrid? If you want to test an old feature with a new one, isn’t the right answer to write a new test that tests both?

I also am worried about the --use-current-setup feature. The purpose of revert_to_defaults() is to give a test a well-known setup where they can start from. Tests need to create a certain setup to test what they need to. If the base PBS configuration is changed out from underneath them, tests could fail. This could lead to test failures where its neither the test’s nor PBS’s fault. This leads to more manual effort to determine why a test failed.

One more question, is the real use case of this feature to save time in setUp()? Right now for every test, we revert back to defaults, just to set it back up. Are we trying to save this time and set some things up once and leave them set up? If this is a use case, you should mention it in the document.

I am uncertain why we would want to revert back to OOB when --use-current_setup is not passed to pbs_benchpress. If we’re going through the trouble of saving configurations now, why not return us back to the configuration we started with?

Bhroam

22

Why do we want to revert to OOB configuration in PBSTestSuite SetupClass?

at example 4 under setUp “-Saves current PBS for first time”, I think you want to say “-Saves current PBS-S1 for first time”

Please note it explicitly that test setUp will take precedence over setUpClass and setUpClass will take precedence over current setup.

Also most of our test has test specific setUp() defined. How is this going to help in your original use case of testing new feature with existing tests?

23

The purpose of this feature to help developer to test their changes/new features (under development) with already existing PTL tests without any changes to testcases. (PP-904). This will help in developer testing and finding out bugs when two features interact with each other.

This is not for production environment. In Travis and other Test environments (in house) we will continue to use PTL with OOB configuration.

24

This will be condition based and it will happen only when user has not mentioned --use-current_setup option.
If user has modified pbs configuration outside PTL and invokes pbs_benchpress without --use-current_setup option then PBSTestSuite SetupClass will revert to OOB configuration.

Yes.

Current PTL revert to OOB configuration then test adds its specific setup on top of OOB configuration.
With this new option PTL will not revert to OOB so user can update pbs configuration and then invoke PTL. Now test will get modified PBS configurations than the OOB and test will add its specific setup on top of modified configuration

25

Thanks Kumar. The last part is not clear from the document. Looking at the table it seems like setUp and setupclass is over-riding current setup. But then I will have the original concern, what if there are conflicting configuration for example, current setUp is enabling something however test setUp is disabling the same attributes. I am assuming since test setUp will be done last so that will take precedence.

26

Thanks @bhroam for your views,
please find my replies below:

“I find this feature interesting, but I am somewhat confused about its purpose. If we use the current configuration instead of OOB to run tests, we’re basically doing a hybrid manual/automated test. The purpose of PTL is to write automated tests, do we want this hybrid? If you want to test an old feature with a new one, isn’t the right answer to write a new test that tests both?”
Sarita ==>
As rightly said by Kumar, the intent is to test the interaction of the PBS features when working together i.e. existing feature tests with new feature enabled or disabled. Of course, the new feature tests need to be written with test focusing on the nuances related to the newly introduced feature. Writing new PTL tests for existing features might lead to multiple versions of the same test with the given new feature on.

Yes,I think you are right with “hybrid tests” that makes it partially manual. Thinking about the hybrid tests; thought comes to my mind that it would be difficult if a bug is caught in this scenario and needs to be reproduced by someone else on a different setup. This makes me feel we need a feature to record and replay PBS configuration settings; which might be taken up as a separate RFE.

“I also am worried about the --use-current-setup feature. The purpose of revert_to_defaults() is to give a test a well-known setup where they can start from. Tests need to create a certain setup to test what they need to. If the base PBS configuration is changed out from underneath them, tests could fail. This could lead to test failures where its neither the test’s nor PBS’s fault. This leads to more manual effort to determine why a test failed.”
Sarita ==>
Again as said by Kumar, this is not for production. This feature is more intended to catch bugs when new features are developed and can also be used for adhoc testing by QA to test interactions of PBS features which are not conflicting. Hence, the validation of test result plays a very important role; since it might be a false pass or fail.

“One more question, is the real use case of this feature to save time in setUp()? Right now for every test, we revert back to defaults, just to set it back up.”
Sarita ==>
The way PBS is brought back to defaults is being changed to load_configuration() method being the way to get PBS back up. revert_to_defaults() would be called only once in setUpClass() to revert to OOB if PBS has anything different (in absence of use-current-setup).

“Are we trying to save this time and set some things up once and leave them set up? If this is a use case, you should mention it in the document.”
Sarita ==>
Yes, I have added this to point B.

“I am uncertain why we would want to revert back to OOB when --use-current_setup is not passed to pbs_benchpress. If we’re going through the trouble of saving configurations now, why not return us back to the configuration we started with?”
Sarita ==>
I think you are referring to case 3 & 4 from the table. The mention of use-current-setup means that there are valid differences in current PBS when compared to OOB which the test runner is authenticating. But case 3 & 4 falls under presence of invalid differences in current PBS that need to be reverted to OOB to start test on a clean environment i.e. the differences the user is not authenticating.

I would like to mention few examples of the use cases here:

  1. Disable scheduler’s throughput mode and run existing tests.
  2. Run existing tests on a varied cluster setup like “comm not running on server node”.
    Ex: In below cluster, pbs.conf needs specific variables like PBS_LEAF_ROUTERS etc.
    Node1 (server+sched+mom)
    Node2 (comm)
    Node3 (mom)
    Node4 (mom)
27

Thanks @anamika,
I have updated S1 in case 4 in the table.
I did not exactly get what you meant by precedence. The setUp, setUpClass & current setup are all summed up together to get he final PBS configurations to be tested upon. I dont think anything takes precedence over any other.
I think I have included the test specific setUp() case as well in the flow chart. (Flows 2, 4 & 6 from left to right) I will think over it more and get back to you

28

Hey @saritakh/@kjakkali
Thanks for answering my questions. I understand a lot better now for most things.

The quoted text doesn’t really answer the question I wanted. From what I read, when --use-current-setup is not passed, PTL will revert to OOB before tests are run. Between each test, it will also return to OOB. After all the tests are complete, it will return to OOB. Everything there makes sense to me but the last part.

My question is why do we revert back to OOB after all the tests are run? Why don’t we save the initial configuration (how PBS is set up before pbs_benchpress is run), and revert back to that when we finish. This way no change in configuration will be made by running pbs_benchpress and testing PBS.

Or am I confused somewhere?

Bhroam

29

Thanks @bhroam,
I think the PBS revertion at last to what we started with; would make PTL save configurations that are not needed by the test. Also, if we do it, should we control it with an option to pbs_benchpress is the question I have.

I see 2 use cases as of now and tried to get below flows for chart flows 3 & 4:

If done:
A. I have some valid manual configurations on my PBS -> I run SmokeTest/Feature test on OOB -> Get back previous configs & Continue my work on my setup
B. I have some invalid manual configurations by mistake on my PBS -> I run SmokeTest/Feature test on OOB -> Will get back my invalid configurations

If not done:
C. I have some valid manual configurations on my PBS -> I run SmokeTest/Feature test on OOB -> Get OOB PBS, I have to redo my previous configurations
D. I have some valid manual configurations by mistake on my PBS -> I run SmokeTest/Feature test on OOB -> Get OOB PBS, a clean PBS

Can you tell me if there is any other advantage of doing this apart from A? or any other use case?

I would want to know everybody’s opinion on this; before going ahead.

30

@All,
A gentle reminder!
Please review and let me know your comments or suggestions.

31

I like Bhroam’s suggestion of reverting back to system config at the very end, there-by preserving the configuration that a user had before they ran pbs_benchpress.

About the EDD, I’m sorry but I still find it a difficult read. A test writer only cares about what configuration their test will get, they don’t care about how setUpClass()/setUp()/tearDown()/tearDownClass() look like. So, while good for people who’d review the code and work on it, these details make the EDD a bit difficult to follow for somebody who’s only looking to understand the externally visible behavior of this change (which is why I had suggested that you break it into external and internal design). But maybe I’m the only one, in which case please ignore this.

32

+1 for @bhroam suggestion about reverting back to user’s config at end

33

Hey @saritakh
I believe you hit the main use case. We don’t want to disturb the user’s PBS configuration by running pbs_benchpress. Thanks for breaking the cases out. I don’t think we should worry about the cases where the user has an invalid PBS setup. We can’t know what is invalid, so we shouldn’t care about it. It is true that vase D can be beneficial. I don’t think it should be a side effect of running pbs_benchpress. Maybe we add a new option to pbs_config? It can be used to revert your PBS setup back to OOB.

Bhroam

34

@All,

With most of us agreeing with Bhroam’s suggestion of saving PBS configurations before test and retaining it back after test is over; I have updated the design document with the same. Please review and let me know of any comments:

Changes done:

  1. Updated A.1 section mentioning the change of retaining PBS configurations
  2. Updated the table and flow chart for the same retainment
  3. Updated A with an additional clarification of effect of presence or absence of “use-current-setup”
35

Thanks @bhroam,

I have updated the design with the same. Also, We already have the option to revert PBS to defaults in pbs_config command:

“pbs_config --revert-config --server”

Usage: pbs_config [OPTION]

-t : comma-separated hosts to operate on. Defaults to localhost
-l : one of DEBUG, INFO, ERROR, FATAL, WARNING

…
–revert-config: revert services to their default configuration
–scheduler: operate on scheduler
–server: operate on server
–mom: operate on MoM
–del-hooks=<True|False>: If True delete hooks. Defaults to True
–del-queues=<True|False>: Delete non-default queues. Defaults to True
…

36

@all,

A gentle reminder!!
Please have a look at the latest changes and let me know of any comments or sign off.

37

Hi @saritakh, it looks good to me. A small comment.

I found section B "changes for setUpClass() conflicting with A.1
“When pbs_benchpress is executed with option ‘use-current-setup’, PBSTestSuite’s setUpClass() method will save the current PBS configurations by calling save_configuration() method. When the option ‘use-current-setup’ is absent, PTL’s PBSTestSuite setUpClass() will revert the current PBS test setup to out of the box by calling all the daemons’ revert_to_defaults() methods, instead of them being called at PBSTestSuite’s setUp().”
–> Should not you need to save existing setUp in setUpclass as well irrespective of --use-current-setup to satisfy A.1 before you revert to OOB.

38

Thanks for catching it @anamika,

I have now reworded the paragraph. Please let me know of any more comments.

39

@All,

A gentle reminder!!
Please review and let me know of any more comments or grant sign off.

40

Thanks @saritakh. I do not have any more comments. It looks good to me.