-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Standardize file names: Fix integration folder names #5298
Standardize file names: Fix integration folder names #5298
Conversation
CHANGELOG.md
Outdated
### Chore & Maintenance | ||
|
||
* `[filenames]` Standardize folder names under `integration-tests/` | ||
([#x](https://github.com/facebook/jest/pull/x)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x = 5298 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, small nits noted
@@ -13,14 +13,14 @@ const path = require('path'); | |||
const runJest = require('../runJest'); | |||
const {cleanup} = require('../utils'); | |||
|
|||
const DIR = path.join(os.tmpdir(), 'jest_global_setup'); | |||
const DIR = path.join(os.tmpdir(), 'jest_global-setup'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In multiple places this need to be changed to: jest-global-setup
(and jest-global-teardown
accordingly too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right you are! Searched and fixed these files:
→ git status -s
M integration-tests/__tests__/global_setup.test.js
M integration-tests/__tests__/global_teardown.test.js
M integration-tests/global-setup/__tests__/setup1.test.js
M integration-tests/global-setup/__tests__/setup2.test.js
M integration-tests/global-setup/__tests__/setup3.test.js
M integration-tests/global-setup/setup.js
M integration-tests/global-teardown/__tests__/teardown1.test.js
M integration-tests/global-teardown/__tests__/teardown2.test.js
M integration-tests/global-teardown/__tests__/teardown3.test.js
M integration-tests/global-teardown/teardown.js
154b3f3
to
5501be2
Compare
@thymikee Both nits addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like these 2 changes are the last for this PR :D
@@ -13,19 +13,19 @@ const os = require('os'); | |||
const path = require('path'); | |||
const runJest = require('../runJest'); | |||
|
|||
const CACHE = path.resolve(os.tmpdir(), 'clear_cache_directory'); | |||
const CACHE = path.resolve(os.tmpdir(), 'clear-cache_directory'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clear-cache-directory
@@ -14,7 +14,7 @@ const runJest = require('../runJest'); | |||
const os = require('os'); | |||
const path = require('path'); | |||
|
|||
const DIR = path.resolve(os.tmpdir(), 'custom_reporters_test_dir'); | |||
const DIR = path.resolve(os.tmpdir(), 'custom-reporters_test_dir'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
custom-reporters-test-dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes. Sorry about that. Both of these are fixed.
Fixes all the folder names in the integration-tests folder to follow Facebook internal file / folder naming conventions. This is the first of several incremental PRs.
5501be2
to
87b3db4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it, thank you!
🎉 thanks. @thymikee thank you for all your help. Next PR should come next Friday. |
You're welcome! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This is the first of many PRs to standardize file names and fix #4969. Per @thymikee guidance I'll be making these changes incrementally. This PR only renames folder names. A future PR will include filenames.
In this PR I:
mv integration_tests integration-tests
integration-tests
Now the folders (not filenames yet) follow standard Facebook practice:
List of commands I ran for all subfolders:
find . -name '*.js' -print0 | xargs -0 sed -i "" "s/integration_tests/integration-tests/g"
I ran this command across.md
,.snap
,.json
,.gitignore
filesFolders I left untouched
__mocks__
,node_modules
,__tests__
, basically anything with a leading__
All the test cases pass. I'm happy to squash the commits as well. Any feedback?