Migrate to jest 2 #1629

AtofStryker · 2020-08-01T17:54:46Z

A rebase of #1587 with browser tests.

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

It's submitted to the dev branch.
When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
All tests are passing: https://github.com/vuejs/vue-test-utils/blob/dev/.github/CONTRIBUTING.md#development-setup
New/updated tests are included

If adding a new feature, the PR's description includes:

A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

This branch introduces moving VTU to jest over mocha, as well as using a few different strategies to run in the browser (discussed below)

AtofStryker · 2020-08-05T01:47:31Z

Wow, incredible work! This would have taken me much longer.

@lmiller1990 Thanks! COVID-19 in the USA has left me with a bit more time than I would usually have available 😅

I do not have the power to add contributors unfortunately. Can we just add the content of df19c1b and get this merged?

All good! I figured it couldn't hurt to ask 😄. We could merge df19c1b here as is as long as you and the team are comfortable with it.

So just to sum up:

we now have tests running in a browser via Jest + Jasmine combo (awesome).

✔️

there are problems with Vue 2.0 and 2.1 tests (you can still use the lib with 2.0 and 2.1, of course). I agree this trade off it worth it.

✔️

I had a look at df19c1b. Reviewing configuration is very difficult but if it's all passing than it should be white_check_mark great job!! merge !== release, and since the actual build process (rollup) did not change there should be little risk.

✔️
Summary seems spot on IMO! There is definitely a lot going on from a configuration standpoint, but the good news is it is pretty similar to what was previously standing. If there is something I can do, such as documentation or other additions, to make the configuration a bit more review-able/digestible, don't hesitate to let me know! Since we are trying to make this improvement last as long as possible (at least for VTU 1.x), we definitely want to make sure other contributors can consume/digest it.

There shouldn't be any immediate risk to these changes, as the bundles are the exact same between branches. I would double check me on that though 😉 . I am in no rush to release or merge by any means. Whatever it takes for us to ship this with confidence and to do it right.

I guess my only question is is there anything blocking merging this right now? It sounds like you have finished the job - while upgrading rollup would be nice, it shouldn't block this PR, right?

The only thing that I believe is of immediate concern is the removed build/release scripts in 28fffd8#diff-b9cfc7f2cdf78a7f4b91a753d10865a2L9 which can be seen here in the file diff. We can add that back in pretty easily, like here before merging.

AtofStryker · 2020-08-05T01:47:55Z

As far as the rollup upgrade is concerned, it isn't necessary for this release, which we could merge it eagerly after just to make it easier to review separate from this chunk of changes. I had some time to look into the upgrade. The good news for us is that virtually nothing has changed for us between the plugin updates and rollup 2.x!

I performed those upgraded changes in AtofStryker@57c4a9c (Sorry, I couldn't help myself 🤣 ) and made a gist of that diff for the team/community review. None of the semver or acorn variables are actually used in the current bundle, and the other changes can be explained by https://github.com/rollup/plugins/blame/master/packages/commonjs/src/helpers.js#L38 and https://github.com/rollup/plugins/blob/master/packages/commonjs/src/helpers.js#L58 / rollup/plugins#206. That being said, when we do perform this upgrade, we should test the build and check for regressions, and I would recommend a minor release just in the rare case we would need to patch.

AtofStryker · 2020-08-05T02:06:14Z

I also have a few other upgrades we can incorporate in separate PRs, including vuepress, core-js 3 (previously blocked by old vuepress and corejs 2.x is deprecated), and chalk (that was definitely the easiest one!). I actually don't think we need jsdom-global or jsdom any longer in the workspace as it ships with jest and was removed here. I might work on an eslint upgrade, and that should really be the final thing we need to do to get VTU 1.x dependencies in a more modern state (for now 🙊 )!

AtofStryker · 2020-08-05T02:07:27Z

The only thing that I believe is of immediate concern is the removed build/release scripts in 28fffd8#diff-b9cfc7f2cdf78a7f4b91a753d10865a2L9 which can be seen here in the file diff. We can add that back in pretty easily, like here before merging.

I can add that commit to this branch, update the description, and mark it ready for review if that is the course of action we want to take 😄

lmiller1990 · 2020-08-08T11:35:59Z

@AtofStryker that'd be great, thanks a lot! Thank you very much for "owning" this feature 👍 excited to drop a tweet about this when we merge it, lmk your Twitter if you have one so I can tag you.

AtofStryker · 2020-08-09T04:20:33Z

packages/test-utils/src/wrapper.js

-              this.vm[key] === data[key] || this.vm.$attrs[key] === data[key]
+              this.vm[key] === data[key] ||
+              // $FlowIgnore : Problem with possibly null this.vm
+              (this.vm.$attrs && this.vm.$attrs[key] === data[key])


I had a few issues in CI recently in regards to this guy in vue 2.3. Looks like I introduced a small regression in VTU for setProps for vue version 2.3 in #1618. The stack trace was a bit difficult to track down, but this should fix the issues seen

After converting to Jest, browser tests were removed due to capatability issues. This reimplements the browser tests with similar techniques. All related webpack/loaders have been updated to use the latest stable releases Chrome/HeadlessChrome is now being used to run these tests over the deprecated PhantomJS Karma and Jasmine are being used as a test runner while leveraging Jest's expect/assertion and mock/stubbing libraries Highly inspired by https://github.com/tom-sherman/blog/blob/master/posts/02-running-jest-tests-in-a-browser.md. 1629

Add back build scripts that were initially removed from jest migration

JSDOM and JSDOM-Global are no longer used with the removal of the mocha tests. JSDOM is still used implicitly by jest.

AtofStryker · 2020-08-09T04:48:29Z

@AtofStryker that'd be great, thanks a lot! Thank you very much for "owning" this feature +1 excited to drop a tweet about this when we merge it, lmk your Twitter if you have one so I can tag you.

@lmiller1990 sure thing! I appreciate you letting me be so involved in this and being so welcome to contributions. I'm a bad technologist and rarely touch social media these days 😆 . But I would definitely appreciate the tag. My twitter is https://twitter.com/AtofStryker.

As far as the PR goes. I made a few changes to address a regression here, but exercised CI after changes and it looks good to me. Build scripts are re added and I removed the jsdom and jsdom-global dependencies as they are no longer used. I think we are ready to rock and roll 🎸

lmiller1990 · 2020-08-11T14:00:23Z

I will get this merged this week.

lmiller1990 · 2020-08-15T15:12:03Z

Ok! It seems like everything is working.

I am having many problems running this locally though. Linting failing...

$ eslint --ext js,vue .
Cannot read property 'range' of null
TypeError: Cannot read property 'range' of null
    at SourceCode.getTokenBefore (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/token-store/index.js:303:18)
    at checkSpacingBefore (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/rules/template-curly-spacing.js:52:42)
    at TemplateElement (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/rules/template-curly-spacing.js:117:17)
    at /Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/util/safe-emitter.js:47:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/util/safe-emitter.js:47:38)
    at NodeEventGenerator.applySelector (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/Users/lachlan/code/projects/vue-test-utils/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
$

Hm.

Edit: Ok, this solved it: babel/babel-eslint#681 (comment)

lmiller1990 · 2020-08-15T15:42:42Z

When I do yarn test locally is hangs on the Karma tests; is this the case for you locally @AtofStryker ? The karma process doesn't close. Is that normal?

AtofStryker · 2020-08-15T16:07:13Z

When I do yarn test locally is hangs on the Karma tests; is this the case for you locally @AtofStryker ? The karma process doesn't close. Is that normal?

@lmiller1990 Yeah that is expected. Karma tests by default run in watch mode, so when changes are made (locally) the tests rerun. If we don't want that behavior, we can just set singleRun to true here

I am having many problems running this locally though. Linting failing...

Glad babel/babel-eslint#681 (comment) solved it. I didn't have issues linting locally, but I also reinstalled from lock after deduping. That also might help, but changing the rule should work just as well

I'll give the changes a pull locally and see if I have any issues

Edit: changes locally look good on my end with the latest commit.

AtofStryker · 2020-08-15T20:00:07Z

BTW @lmiller1990 were you having any other issues or was it just the linting?

lmiller1990 · 2020-08-16T00:29:17Z

Everything else works great. I am guessing singleRun is somehow set to true on CI (or it would hang forever)?

It might be good to have it set locally by default (can you make that change?) I feel like other people contributing will have the same problem I just did.

Other than that, happy to merge and do a release!

Set singleRun option in karma to true locally and in CI so the test process always terminates when finished

AtofStryker · 2020-08-16T00:45:38Z

Everything else works great. I am guessing singleRun is somehow set to true on CI (or it would hang forever)?

@lmiller1990 correct. The config is utilizing the circle var process.env.CI, which Circle sets to true when running in CI.

It might be good to have it set locally by default (can you make that change?) I feel like other people contributing will have the same problem I just did.

Definitely! On revisiting, the watching behavior was probably a poor choice on my end 😆 . Definitely don't want others to run into snags or problems when contributing. We can just set singleRun: true by default so we don't have to worry about any hanging. I have implemented in afad86e

Other than that, happy to merge and do a release!

Sweet! We should be good to go then! 🎉 Once we get this merged in, I will open a few other PRs for some other library upgrades in, such as vuepress, eslint, rollup, etc. They are relatively minimal compared to this PR. Hopefully these changes allow for less cognitive overhead and context switching between the VTU 1.x and 2.x workspaces for contributors and the team 😃

lmiller1990 · 2020-08-16T05:59:16Z

It's merged!!!! 🎉 thanks!

lmiller1990 · 2020-08-17T12:46:29Z

It's out.

AtofStryker force-pushed the migrate-to-jest-2 branch 3 times, most recently from aa7852f to 68eddc5 Compare August 1, 2020 19:37

lmiller1990 added 27 commits August 1, 2020 20:54

refactor: use jest

9c2e68f

refactor: strip flow types when running tests with jest

42b50c9

refactor: update attachTo and destroy specs

58973bf

refactor: continue moving to jest

c7b2d1e

refactor: update tests

de51777

refactor: update tests

e09a8ad

refactor: update tests

bd83440

refactor: continue jest migration

90d7118

refactor: continue jest migration

9afa062

refactor: remove sinon

523e8bc

refactor: update tests to use jest

0fe0eb0

refactor: update tests to use jest

a992654

refactor: add jsx plugin and update scopedSlots spec

b0aba13

chore: add missing deps

e179b92

test: add class component and update stubs tests

623adc2

test: update tests

bf67d57

test: migrate more tests

97207fa

test: continue migration

e92f965

test: refactor set methods

97e6270

test: finish refactor set methods

6c7c1e4

test: refactor trigger

d8517d0

test: refactor wrapper-array specs

08d4848

test: update config.spec.js

495e127

test: update mount specs

f29fbe5

test: use spyOn

cdce1e4

test: migrate tests

a268053

test: placeholder to prevent false failure

711017b

AtofStryker force-pushed the migrate-to-jest-2 branch 4 times, most recently from c119513 to 58b257f Compare August 9, 2020 04:21

AtofStryker commented Aug 9, 2020

View reviewed changes

AtofStryker added 3 commits August 9, 2020 00:26

chore(scripts): re add missing build scripts

3681110

Add back build scripts that were initially removed from jest migration

chore(jsdom): remove JSDOM and JSDOM-Global

0eb83a1

JSDOM and JSDOM-Global are no longer used with the removal of the mocha tests. JSDOM is still used implicitly by jest.

AtofStryker force-pushed the migrate-to-jest-2 branch from 58b257f to 0eb83a1 Compare August 9, 2020 04:30

AtofStryker marked this pull request as ready for review August 9, 2020 04:48

chore: update eslintrc

6cbfadf

chore(karma.config.js): set singleRun to true for all runtimes

afad86e

Set singleRun option in karma to true locally and in CI so the test process always terminates when finished

lmiller1990 merged commit 60c1bce into vuejs:dev Aug 16, 2020

This was referenced Aug 16, 2020

build(rollup): upgrade rollup from 1.x to 2.x #1645

Merged

Upgrade Vuepress, Flow, and misc #1646

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to jest 2 #1629

Migrate to jest 2 #1629

AtofStryker commented Aug 1, 2020 •

edited

Loading

AtofStryker commented Aug 5, 2020

AtofStryker commented Aug 5, 2020 •

edited

Loading

AtofStryker commented Aug 5, 2020

AtofStryker commented Aug 5, 2020

lmiller1990 commented Aug 8, 2020

AtofStryker Aug 9, 2020

AtofStryker commented Aug 9, 2020

lmiller1990 commented Aug 11, 2020

lmiller1990 commented Aug 15, 2020 •

edited

Loading

lmiller1990 commented Aug 15, 2020 •

edited

Loading

AtofStryker commented Aug 15, 2020 •

edited

Loading

AtofStryker commented Aug 15, 2020

lmiller1990 commented Aug 16, 2020 •

edited

Loading

AtofStryker commented Aug 16, 2020

lmiller1990 commented Aug 16, 2020

lmiller1990 commented Aug 17, 2020

Migrate to jest 2 #1629

Migrate to jest 2 #1629

Conversation

AtofStryker commented Aug 1, 2020 • edited Loading

AtofStryker commented Aug 5, 2020

AtofStryker commented Aug 5, 2020 • edited Loading

AtofStryker commented Aug 5, 2020

AtofStryker commented Aug 5, 2020

lmiller1990 commented Aug 8, 2020

AtofStryker Aug 9, 2020

Choose a reason for hiding this comment

AtofStryker commented Aug 9, 2020

lmiller1990 commented Aug 11, 2020

lmiller1990 commented Aug 15, 2020 • edited Loading

lmiller1990 commented Aug 15, 2020 • edited Loading

AtofStryker commented Aug 15, 2020 • edited Loading

AtofStryker commented Aug 15, 2020

lmiller1990 commented Aug 16, 2020 • edited Loading

AtofStryker commented Aug 16, 2020

lmiller1990 commented Aug 16, 2020

lmiller1990 commented Aug 17, 2020

AtofStryker commented Aug 1, 2020 •

edited

Loading

AtofStryker commented Aug 5, 2020 •

edited

Loading

lmiller1990 commented Aug 15, 2020 •

edited

Loading

lmiller1990 commented Aug 15, 2020 •

edited

Loading

AtofStryker commented Aug 15, 2020 •

edited

Loading

lmiller1990 commented Aug 16, 2020 •

edited

Loading