Please review the design document for pbs_benchpress to run from non-tests directory
After PTL rpm Installation, the user has to cd to tests dir to execute test cases. We have -f option in pbs_benchpress but it supports only file. But we cannot use it to run a whole bunch of different tests.
when user runs pbs_benchpress -t testcase, it should do a path search – search current directory, next search test case in installed location.
Please let me know if any comments or suggestions.
Thank you for sharing the design doc.
A few comments -
- Design page should have a link to the forum discussion.
- There are many typos - like dircetory, pbs_benhcpress, Serached etc.
- The statement “pbs_benhcpress -i option is also now can be run…” needs rephrasing.
- Examples would help in better understanding.
Hi @prakashcv13, Thanks for the review comments.
I have updated EDD with examples and corrected typos.
Please have a look at it.
@shilpakodli I would suggest to rename variable name from
How about PTL_TESTS_PATH?
PTL_TESTS_PATH seems to be more reasonable since path represents how to reach a filesystem object.
both are alright, but usually “PATH” would support more than one directories, but here we are supporting only one. I would go for PTL_TESTS_DIR.
If we plan to expand the support to multiple directories in the future, then PTL_TESTS_PATH would be better.
I agree with @prakashcv13.
I agree with @prakashcv13. As we are supporting one test dir so, I prefer PTL_TESTS_DIR over PTL_TESTS_PATH
Thanks everyone. I will rename PTLTESTS to PTL_TESTS_DIR as we are supporting single tests directory as of now.
I’m not sure I understand. You mean PATH can be set to path1:path2:path3 ? if yes then that’s true for any environment variable right?
@shilpakodli, In scenario2
We are considering ‘PTL_TESTS_DIR’ dir as last option, since user has set it we should consider it before 'installed test’dir.
@agrawalravi90 - I am assuming, this question was for me. I will try to be more clear this time -
Usually PATH would mean we support path1:path2:path3.
As of now we are not supporting this, but if we plan to support this in the future, we should use PTL_TESTS_PATH or for that matter PTL_USER_TESTS_PATH.
If we do not plan to support multiple directories even in future, we should use PTL_TESTS_DIR or PTL_USER_TESTS_DIR.
I am leaned towards including “USERS” in the name as we already have implicit paths that pbs_benchpress would search.
@shilpakodli - could you please also add some detail on the motivation behind adding this support. Based on that we could decide/discuss on the order in which pbs_benchpress should search for the tests.
@shilpakodli - I have a question. What would happen if the user sets the environment variable to have more than one directory?
Ok, so you are saying that there would be an expectation from the users that PTL_TESTS_PATH will accept multiple cascaded paths. I see your point, but I don’t think it’ll be an issue, we just need to document that it accepts only 1 directory. But if you strongly feel about it, then I’m ok with PTL_TESTS_DIR.
I don’t think we need to add the word USERS in there, we don’t do that for any of the other PBS environment variables. Let’s keep it short, otherwise it’ll be a pain to use.
@prakashcv13 - If we give multiple paths in PTL_TESTS_DIR then it won’t be able to search test case since, I am passing entire value of PTL_TESTS_DIR as tests directory path.
@shilpakodli - thank you - how about adding this as scenario 4?
@agrawalravi90 - I agree that adding “USER” to the name would make it longer. The design is clear about the order in which we would be looking for tests, so “USER” is implicit.
@shilpakodli - I agree with @kjakkali wrt the order. There is a possibility that a user writes a testsuite with a name identical to one in PTL installation directory, the user would expect his version of the testsuite to run.
Sure, we can have one scenario of having multiple paths in PTL_TESTS_DIR where it will throw error by saying “Unknown testsuite/testcase”
@shilpakodli : The latest EDD looks good. I sign off.