-
Notifications
You must be signed in to change notification settings - Fork 29.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: More descriptive name for test file names #19592
Labels
good first issue
Issues that are suitable for first-time contributors.
test
Issues and PRs related to the tests.
Comments
Even though there is no hard limit for the file name length, it is usually a good idea to keep them as short as reasonably possible. How about |
@tniessen That makes sense, this way it keeps the file names concise and also indicates what kind of methods the file tests for |
lpinca
added
test
Issues and PRs related to the tests.
good first issue
Issues that are suitable for first-time contributors.
labels
Apr 2, 2018
@rajdhandus sure. |
thanks.. |
2 tasks
rajdhandus
added a commit
to rajdhandus/node
that referenced
this issue
Apr 6, 2018
test-process-geteuid-getegid.js will be renamed to test-process-euid-egid.js test-process-setuid-setgid.js will be renamed to test-process-uid-gid.js Fixes: nodejs#19592
targos
pushed a commit
that referenced
this issue
Apr 12, 2018
test-process-geteuid-getegid.js is renamed to test-process-euid-egid.js. test-process-setuid-setgid.js is renamed to test-process-uid-gid.js. PR-URL: #19765 Fixes: #19592 Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
good first issue
Issues that are suitable for first-time contributors.
test
Issues and PRs related to the tests.
I was going through the following test files
test-process-geteuid-getegid.js
andtest-process-setuid-setgid.js
I was wondering if the filenames could include the name of all the methods that are being tested so
test-process-geteuid-getegid.js
could be renamed totest-process-geteuid-getegid-seteuid-setegid.js
test-process-setuid-setgid.js
could be renamed totest-process-getuid-getgid-setuid-setgid.js
I had a little confusion when locating these files and I thought maybe this change would help. Also, I read the guidelines on naming test files and it allows for listing the methods being tested and additional information where needed.
There are test files with over 60 characters eg.
test-timers-socket-timeout-removes-other-socket-unref-timer.js
,test-process-exception-capture-should-abort-on-uncaught-setflagsfromstring.js
so I understand the renaming would not violate any character limit restrictions on test file namesIf the above change isn't possible I was thinking for the sake of consistency, the test files could be renamed to
test-process-getuid-getgid.js
andtest-process-getegid-geteuid.js
. I was initially confused for a bit when I found thetest-process-geteuid-getegid.js
but not a correspondingtest-process-getuid-getgid.js
The text was updated successfully, but these errors were encountered: