Skip to content
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 filenames in packages/jest circus #7301

Merged
merged 15 commits into from Oct 31, 2018
Merged

Standardize filenames in packages/jest circus #7301

merged 15 commits into from Oct 31, 2018

Conversation

andrew-pyle
Copy link
Contributor

Summary

References Standardize file naming #4969.

Updates naming in package jest-circus. Uses conventions listed in #4969 (comment):

  • Files that primarily export types, objects or classes should use CapitalizedFileNames.js and should mirror what’s inside 1:1.
  • Files that export a single function should have the function name with camelCase in it.
  • Folder names should use dashes, unless they are special folders.

Decisions

  • Test & Mock files retained their name, but converted it to camelCase
  • formatNodeAssertErrors.js had an export of the format
    export default (parameter: type...) => {}
    which was converted to the following to let the filename mirror the exported function.
    const formatNodeExceptionErrors = (parameter: type...) => {}
    // rest of file
    export default formatNodeExceptionErrors
  • error_handlers.js was renamed globalErrorHandlers.js to more closely match the two exported functions: injectGlobalErrorHandlers & restoreGlobalErrorHandlers
  • Thelegacy-code-todo-rewrite directory was renamed, but the Javascript files inside were not renamed. It seems like a good way to help get them rewritten?

Test plan

Output of yarn run test on master and standardize filenames in packages/jest circus branches was the same, with the exception of one test which failed on master, but not this branch. Not sure what would cause that.

FAIL  examples/manual-mocks/__tests__/lodashMocking.test.js
  ● if lodash head is mocked

    expect(received).toEqual(expected)

    Expected value to equal:
      5
    Received:
      2

      4 | 
      5 | test('if lodash head is mocked', () => {
    > 6 |   expect(lodash.head([2, 3])).toEqual(5);
        |                               ^
      7 | });
      8 | 

      at Object.toEqual (__tests__/lodashMocking.test.js:6:31)

jest-tests-master.txt
jest-tests-standardize-filenames-in-packages-jest-circus.txt

- File, Function & Imports renamed
- `export default (...` changed to
  `const formatNodeAssertErrors = (...` with
  `export default formatNodeAssertErrors` at end of file
- Rename file, function, imports, mocks, & tests
- Forgot snapshot when renaming afterAll.test.js
- Rename snapshot to match test filename
- Added "it" to name to match CircusItTestError.test.js
- Rename file
Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Still some files left to rename

@@ -10,10 +10,10 @@

'use strict';

import {runTest} from '../__mocks__/test_utils';
import {testUtils} from '../__mocks__/testUtils';
Copy link
Collaborator

Choose a reason for hiding this comment

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

runTest was a good name, please revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rename testUtils to runTest. Leave the filename testUtils?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yup, leave it was :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which files are left to rename?

Copy link
Collaborator

Choose a reason for hiding this comment

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

screenshot 2018-10-30 at 23 03 18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good. I'll rename those as well

- jest_adapter.js      -> jestAdapter.js
- jest_adapter_init.js -> jestAdapterInit.js
- jest_expect.js       -> jestExpect.js
- Fixed imports within directory and in runner.js
@thymikee thymikee merged commit 2f32100 into jestjs:master Oct 31, 2018
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants