Hi All,
Please review the design document created at:
https://pbspro.atlassian.net/wiki/spaces/PD/pages/395476993/PP-1278+Need+a+skip+decorator+in+PTL+which+will+skip+the+test+without+any+condition.
Please do let me know of any comments or suggestions.
Thanks and Regards,
Sanket
Thanks for taking this up Sanket.
The design document is pretty terse, did you want to add any technical details? According to the new design doc guidelines (https://pbspro.atlassian.net/wiki/spaces/DG/pages/293076995/PBS+Pro+Design+Document+Guidelines), we are now encouraging people to document the âinternalâ design as well in the same design doc. So, if you wanted to add any technical details, please do so here itself.
About the existing contents of the doc: I think the interface is â@skipâ and not âskip()â, so Iâd recommend changing the interface line to say that instead. The document looks fine otherwise.
Hi @agrawalravi90, I have made the changes according to your comments, please have a look.
Iâm not sure I see the need for this decorator. Itâs one line to add @skip. Itâs one line to add self.SkipTest(). At least with the second, you get to add a message on why you are skipping the test.
What benefit do we gain by the decorator?
@bhroam, I might be wrong about this, but I think one of the benefits is performance, you donât even run setup if the decorator skips it.
Skipping setUp() would be a benefit. Any chance we can add a skip message to the decorator?
I think decorators that check for a certain condition and then pass the test would be more useful.
Some examples:
@two_moms that checks the number of moms and skips the test if there are less than two@cray, @linux, @windows that would skip tests unless they are run on that platformYes, we can add arguments to decoratorsâŚ
@minghui I definitely liked you idea about @cray, @linux and @windows, but we are trying to combine all skip decorator in one so that end user doesnât have to remember all different decorators
I donât see why not, it should be pretty easy to add an optional message argument to skip(), right @borlesanket ?
I agree with Hiren on this one, there might be an endless list of reasons why somebody might wanna skip a test. Sanket should correct me, but I think the @skip decorator is supposed to be used for tests which should be skipped temporarily, unconditionally, until some PTL/PBS bug is fixed. We could also just file a ticket to write the same test after the bug is fixed, but usually those tickets never get done. Sanket and Hiren should correct me. Also, @borlesanket, it might be useful to document the use cases for this feature as well for questions like this.
@agrawalravi90 Yes, current use case of @skip is what you told. But future plan is to expand @skip with set of arguments which will do conditionally skip⌠like for example add argument called platform which will take value like cray, linux and windows then @skip will do platform match and based on result it will skip testâŚ
In other words what @minghui suggested for @cray will become @skip(platform='cray'), @linux will become @skip(platform='linux') and @windows will become @skip(plarform='windows')
Just a thought for the future, we might wanna add an option to the skip decorator where the user can pass a custom function as the argument (e.g: @skip(func=is_valid_config)) which would be called by the skip decorator and should return True or False to the decorator, on the basis of which it will skip the test. This will allow users to write any custom skip function which can be passed as argument to the skip decorator. Or maybe this is too cumbersome? anyways, just a wild thought.
@agrawalravi90 not âmightâ, but we want to add support for custom function as per offline discussion about one generic decorator 
And will watch all custom function when any user adds it and if will see is that custom function can be made generic and can be added in @skip decorator via new parameter, will do it.
For example:
Assume that @skip decorator is implementation with only two argument for now, one for message and one for custom function.
And now user used @skip with custom function called âis_platform_crayâ as custom functionâŚ
Well we have new custom function which can be made generic and can be added via new argument in @skip so will add new argument to @skip called âplatformââŚ
Hope you point my pointâŚ
Great, yes, I like the approach, good to know thatâs what we are going with already!
Adding @skip decorator will be useful for skipping tests temporarily while testing based on the setup or some bugs which are blocking the test case. This is a good idea. There may be more such scenarios where this can be effectively used, may be such examples should be added to the EDD?
@bhroam, yes this will skip the setup and also we can pass the reason for the skip.
@agrawalravi90, @minghui, yes as @hirenvadalia said we will be adding more functionalities in future.
@neha.padole I have added one more example in doc, in my opinion those are the two scenarios where we can make use of this decorator. Please let me know if you have any apart from this in your mind.
Thanks for adding the reason to @skip, @borlesanket. Since in the future we are planning on adding named conditionals, should we start now naming reason? @skip(reason=âfooâ)? Or is this like a normal python function where you can pass some location dependent parameters and other named parameters?
Bhroam
@bhroam, yes naming the reason will be useful from test case writers perspective in future. I have made changes accordingly. Also, I have updated couple of points which I forgot to do last time.
Hi All, please provide your comments if any or sign off.