PP-519: Modify skipOnCray decorator in PTL to skip test on Cray, Cray ALPS simulator and Cray build


#1

Hi All,

Please review the design document created for "skipOnCray decorator to handle test case skip on Cray"
https://pbspro.atlassian.net/wiki/spaces/PD/pages/53366181/PP-519%3A+Modify+skipOnCray+decorator+in+PTL+to+skip+test+on+Cray%2C+Cray+ALPS+simulator+and+Cray+build

Please do let me know of any comments or suggestions.

Thanks and Regards,
Sanket


#2

@borlesanket, could you give some examples? Are the comma-separated parameters supposed to be enclosed in single quotes? Will there be an error for invalid parameters?


#3

@vccardenas, please find some of the examples below:

case 1. Skip on actual Cray cluster
@skipOnCray(‘cray’)

case 2. Skip on Cray ALPS simulator
@skipOnCray(‘craysim’)

case 3. Skip on cray build
@skipOnCray(‘craybuild’)

case 3: Skip on both actual Cray and simulator
@skipOnCray(‘cray,craysim’)

case 4: Skip on all actual Cray, Cray ALPS simulator and Cray build
@skipOnCray(‘cray,craysim,craybuild’)

Regarding comma separated values, they should be enclosed in single quotes as we see in case 3 and case 4 above.
Also error will be raised for invalid parameters.


#4

@borlesanket, thanks for the examples. Your Synopsis might need a reference to Cray build. Please state that error will be raised for invalid parameters.


#5

@vccardenas, I have incorporated the comments, please have a look.


#6

@borlesanket : Looks good to me.


#7

@borlesanket, thanks for the changes - looks good to me.


#8

I was about to ask for usage examples, then I noticed I’m not the first to ask, however, the examples only ended up in the forum. Can you please add some examples to the actual design doc? Thanks!

Also, based on the examples in the forum, it seems that there should be no white space between the options? And also a single quote around all of the parameters? If these are requirements, then they should be listed in the external design.


#9

@lisa-altair, I have added some examples to doc. Regarding white space it does not matter and about the single quote I have mentioned in first bullet. Please have a look.


#10

I remember I have given such comments earlier and still feel that we should make the decision of whether or not to run a test more generic.

It seems odd that skipOnCray() decorator without arguments will now throw error when the function name itself mentions cray in it.

We should either make it generic like “skipOn({platform:[‘cray’,‘craysim’], build:‘craybuild’})” or do what is mentioned in https://pbspro.atlassian.net/browse/PP-695 which is generic in nature.

what do others think?


#11

Good idea @arungrover! Generic is always good! I hate for work to be done for a specific platform, only to have to change what was done later when it’s implemented more generically. We should start with the generic implementation whenever possible.