Hi All,
Please review the design document for pbs_benchpress to run from non-tests directory
https://pbspro.atlassian.net/wiki/spaces/PD/pages/1319829507/Enhancement+of+pbs+benchpress+to+run+from+non-tests+directory
Objective:
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.
Thanks,
Shilpa
Hi @shilpakodli,
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.
Thanks,
Prakash
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 PTLTESTS
to PTL_TESTS_DIR
.
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.