Skip to content
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

fix test config #1606

Merged
merged 2 commits into from
Aug 30, 2019
Merged

fix test config #1606

merged 2 commits into from
Aug 30, 2019

Conversation

sirugh
Copy link
Contributor

@sirugh sirugh commented Aug 26, 2019

Description

OK here's why it's broken.

jest.config.js uses inPackage to create a path to things in configs. It's a fancy helper for resolving paths. The specific problem is that __mocks__/fileMock.js does not exist in venia-concept. I think that jest caches some things after running which is why you may or may not see this issue but I assure you it is a problem. If you do a clean checkout into a new directory you should see it happen.

So, we can solve this one of two ways.

  1. Any test config using testVenia hardcodes a path to __mocks__/fileMock.js in venia-ui.

  2. Create the __mocks__/fileMock.js in venia-concept.

Personally I don't care what we chose but it is broken and we should fix it.

Verification

  1. yarn test:dev/jest --watch doesn't break.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Aug 26, 2019

Fails
🚫 Missing "Verification Steps" section. Please add it back, with detail.
🚫

No linked issue found. Please link a relevant open issue by adding the text "closes #<issue_number>" in your PR.

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

If your PR is missing information, check against the original template here. At a minimum you must have the section headers from the template and provide some information in each section.

Generated by 🚫 dangerJS against 563b3ff

@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Aug 28, 2019
@supernova-at supernova-at self-assigned this Aug 28, 2019
@supernova-at
Copy link
Contributor

supernova-at commented Aug 28, 2019

It seems to be working for me on develop?

git fetch
git checkout develop
git pull
yarn clean:all
yarn install
yarn test:dev (pressed a to "run all tests")

Result:

Test Suites: 1 skipped, 196 passed, 196 of 197 total
Tests:       14 skipped, 1068 passed, 1082 total
Snapshots:   131 passed, 131 total
Time:        24.02s, estimated 41s
Ran all test suites in 7 projects.

@sirugh
Copy link
Contributor Author

sirugh commented Aug 28, 2019

Well... that's weird. It's working for me too. I don't know how to repro!

@sirugh
Copy link
Contributor Author

sirugh commented Aug 28, 2019

Wait! I just had it happen... This is my terminal history and the output:

Image from Gyazo

@supernova-at
Copy link
Contributor

supernova-at commented Aug 29, 2019

Man, I'm still not getting the error on develop.

Your console output includes "Determining test suites to run..." but mine never does 🤔

@revanth0212
Copy link
Contributor

Happened to me till yesterday when I tried to use jest in watch mode. Unfortunately, I am not able to reproduce it now. Weird.

@sirugh
Copy link
Contributor Author

sirugh commented Aug 29, 2019

Checked out the repo anew and ran yarn test:dev caused it to fail again for me. lol

@vercel vercel bot requested a deployment to staging August 29, 2019 22:13 Abandoned
@supernova-at
Copy link
Contributor

Verification steps complete ✅

@dpatil-magento dpatil-magento merged commit b32725c into develop Aug 30, 2019
@dpatil-magento dpatil-magento deleted the rugh/fix-test-config branch August 30, 2019 14:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
pkg:venia-concept version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants