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

fix(testing): handle snapshot filepaths for jest #3282

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

rwaskiewicz
Copy link
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

cf42114 created a regression where files named e2e.ts.snap would not be picked up correctly by the test runner

What is the new behavior?

this commit updates the testing regex in our jest base configuration to
handle files such as testing snapshots - those that contain 'spec.ts'
(or a similarly acceptable file extension for tests), but do not end
with that extension. For example, my-component.test.ts.snap

this fixes a regression introduced in cf42114

Does this introduce a breaking change?

  • Yes
  • No

Testing

I pulled this branch, built this branch, and generated a tarball: npm ci && npm run build && npm pack

Within Ionic Core, I installed my tarball in ionic-framework/core from the build command above, and verified the correct number of tests were run (verifying against #3277 results from late last week)
Screen Shot 2022-03-14 at 12 43 27 PM

I also tested this in a local (small) stencil project.

First, let's create a project and verify we can run tests:

npm init stencil@latest component snap
cd snap
npm i && npm t

Those tests pass, let's add a .snap file to verify the issue:

touch src/components/my-component/my-component.spec.ts.snap
npm t

Returns a warning that we found a test file without any tests in it. Let's install our tarball to verify it fixes it:

npm i <TARBALL>
npm t

🎉

this commit updates the testing regex in our jest base configuration to
handle files such as testing snapshots - those that contain 'spec.ts'
(or a similarly acceptable file extension for tests), but do not end
with that extension. For example, `my-component.test.ts.snap`

this fixes a regression introduced in cf42114
@rwaskiewicz rwaskiewicz requested a review from a team March 14, 2022 16:50
@rwaskiewicz rwaskiewicz merged commit d164dba into main Mar 16, 2022
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/fix-test-regex-ending branch March 16, 2022 16:05
rwaskiewicz added a commit that referenced this pull request Mar 24, 2022
this commit updates the testing regex in our jest base configuration to
handle files such as testing snapshots - those that contain 'spec.ts'
(or a similarly acceptable file extension for tests), but do not end
with that extension. For example, `my-component.test.ts.snap`

this fixes a regression introduced in cf42114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants