Skip to content

test: rename regression tests with descriptive file names (pt. 7) #19668

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

ryzokuken
Copy link
Contributor

@ryzokuken ryzokuken commented Mar 29, 2018

Rename the tests appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the tests conform to the standard test structure

Refs: #19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

Checklist

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 29, 2018
@lpinca
Copy link
Member

lpinca commented Mar 29, 2018

@lpinca lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2018
@ryzokuken
Copy link
Contributor Author

@lpinca let me just add one more commit in a jiffy.

@lpinca
Copy link
Member

lpinca commented Mar 29, 2018

Sure.

@lpinca lpinca removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2018
@ryzokuken
Copy link
Contributor Author

@lpinca done, thanks! Please take a look at the new one, and if it looks good to you, I'll squash the two.

'use strict';
require('../common');

// This test ensures that the 'exit' event handler is not flaky, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit: I'd simplify like "This test ensures that no asynchronous operations are performed in the 'exit' event handler".

@lpinca lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2018
Rename test-fs-truncate-nodejsGH-6233 to test-fs-truncate-clear-file-zero
Rename test-process-exit-nodejsGH-12322 to test-process-exit-flaky-handler

Refs: nodejs#19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
@lpinca
Copy link
Member

lpinca commented Mar 29, 2018

@lpinca
Copy link
Member

lpinca commented Apr 1, 2018

Landed in 107b067.

lpinca pushed a commit that referenced this pull request Apr 1, 2018
- Rename test-fs-truncate-GH-6233 to test-fs-truncate-clear-file-zero
- Rename test-process-exit-GH-12322 to test-process-exit-handler

PR-URL: #19668
Refs: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@lpinca lpinca closed this Apr 1, 2018
targos pushed a commit that referenced this pull request Apr 2, 2018
- Rename test-fs-truncate-GH-6233 to test-fs-truncate-clear-file-zero
- Rename test-process-exit-GH-12322 to test-process-exit-handler

PR-URL: #19668
Refs: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Apr 4, 2018
BethGriggs pushed a commit that referenced this pull request Dec 4, 2018
- Rename test-fs-truncate-GH-6233 to test-fs-truncate-clear-file-zero
- Rename test-process-exit-GH-12322 to test-process-exit-handler

PR-URL: #19668
Refs: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants