Hi @agrawalravi90, I have updated the summary.
Ok, so from what I understand this will allow test writers to provide default values for some -p parameters. If that’s correct, then can’t the test just check the -p values and use defaults inside the test if the parameters were not passed via pbs_benchpress -p ?
@agrawalravi90 That’s where it started and what was done eariler . But we want to make it more userfriendly for test developer than he keeps verifying for all the test variables if it is present in -p or else use default and Some requirements were that person who will execute the test should know about the test configurable parameters before hand running the test or going through the test.
So now we have pbs_benchpress -i now showing test doc string with variable test parameters.
Ok, that sounds like good reasons to do this, and again something that you should mention under a motivation section in the doc. So, please do mention these 2 use cases as well in the doc.
@agrawalravi90 updated use cases.
"Test Framework should provide an interface to modify the test parameters. "
That already exists right? via the -p option?
Also, can the -p option take any custom test parameter? e.g - if a test exposes a configurable parameter called ‘num_jobs’, can one pass pbs_benchpress -p num_jobs=100 ? Does the -p infrastructure need to be enhanced to support any custom test parameter? Or do we want to standardize a set of test parameters that PTL will support and not allow any random parameters?
"Test Framework should provide an interface to modify the test parameters. "
That already exists right? via the -p option?
Vishesh -> Yes it exists.
Also, can the -p option take any custom test parameter? e.g - if a test exposes a configurable parameter called ‘num_jobs’, can one pass pbs_benchpress -p num_jobs=100 ?
Vishesh -> Yes it can.
Does the -p infrastructure need to be enhanced to support any custom test parameter? Or do we want to standardize a set of test parameters that PTL will support and not allow any random parameters?
Vishesh -> No we don’t need to enhance . We cannot standardize , random params should be allowed because the parameters should be test specific.
@arungrover @kjakkali @saritakh As majority of people feel that having in -p is more beneficial for test runner. In the perspective of a test runner having it in -p option will be more useful as he does not need to change the JSON file everytime.
When a parameter is mentioned in -p as -p a=value, As @arungrover suggested we can save it as TestSuite.a=value and we can use it in test case as self.conf[‘TestSuite.a’] with having a guideline to not have duplicate param names inside a testsuite.
@visheshh : I am fine with both approaches. my main concerned was user easiness.
Can this decorator be applied at test suite level?
@kjakkali : As these are test specific parameters and will be random across the tests. I don’t think we will be requiring this at a testsuite level.
Thanks for clarifying Vishesh.
In that case, please remove that as a use case from the doc as this is already supported.
I think you can remove the “Summary” section and just move the test parameters example to the Use Cases section.
" Synopsis : @testparams is a decorator to specify default values of test specific parameters"
Maybe say: @testparams is a decorator to specify tunable test parameters for a test and their default values
The rest looks fine to me.
Please mention that in the doc
I agree with the mentioned approach of ‘-p’.
Regarding the test suite level decorator specifications, I guess we should go ahead with the behavior we have right now for other decorators where if mentioned at test suite level it is common to all test cases and whichever test case accesses this value gets the same. If mentioned at both test suite and test case level, test case level value will take precedence over test suite level value when not mentioned in ‘-p’; when -p value is mentioned that would be the effective value alone irrespective of defaults at test case / test suite.
@saritakh I thought through this .
I think for the same reason we are encouraging to keep variables distinct across test cases we should not do this.
In case 1 persons adds 2 tests for job submission and keeps num_jobs same across tests and declares at testsuite level and next person wanting to add some server restart tests now wants to submit millions of jobs and uses num_jobs and modifies num_jobs then there will be collisions and confusions.
In a test suite when number of test cases keeps on increasing the confusion increases whether to declare at test suite level or test case level.
So its better we keep them as specific as possible to test cases.
Hi @arungrover @agrawalravi90 @kjakkali @saritakh
Please provide comments if any before freezing the design.
Thanks Vishesh, I agree about having the decorator only at test case level.
The design looks good to me.
Looks fine, thanks for making the changes
@visheshh : I sign off.
I’m fine with the proposed changes @visheshh. Thanks for working on it.