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.

Sign up for GitHub

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

Jump to bottom

Assess test coverage of javascript code/packs [13,13,1] #2380

Closed
stenington opened this issue Feb 13, 2020 · 7 comments
Closed

Assess test coverage of javascript code/packs [13,13,1] #2380

stenington opened this issue Feb 13, 2020 · 7 comments
Assignees
@shaun-technovation
Labels
tech debt

Comments

@stenington
Copy link
Contributor

stenington commented Feb 13, 2020 •
edited by rsgonzal
Loading

From #2166 and following on #2363, we need to check our test coverage on the javascript getting built into packs before upgrading webpack. If it's good, that'll help during an upgrade. If it's not good, we would need to increase coverage before attempting the upgrade.

QA Steps

This one is on staging, but there is no front-end piece to this one, so I don't think any manual QAing is necessary. 😃
@stenington stenington changed the title Assess test coverage of javascript code/packs Assess test coverage of javascript code/packs [?,8,?] Feb 13, 2020
@stenington stenington changed the title Assess test coverage of javascript code/packs [?,8,?] Assess test coverage of javascript code/packs [13,8,1.63] Feb 13, 2020
@stenington stenington mentioned this issue Feb 14, 2020
Closed
@rsgonzal rsgonzal added it-17 and removed it-16 labels Feb 14, 2020
@stenington
Copy link
Contributor Author

Outcomes could be: run a coverage tool, get some numbers, identify gaps. Do a pairing session with other dev so we both have some sense of what's going on in this area.

@hellafitz hellafitz changed the title Assess test coverage of javascript code/packs [13,8,1.63] Assess test coverage of javascript code/packs [13,13,1] Feb 26, 2020
@shaun-technovation shaun-technovation self-assigned this Feb 28, 2020
@shaun-technovation
Copy link
Contributor

Documenting some simple/obvious things that took me on quite a ride yesterday! 🎢

npm ci - this is supposed to give you reproducible builds (I just discovered this command yesterday)

https://docs.npmjs.com/cli/ci
https://stackoverflow.com/questions/52499617/what-is-the-difference-between-npm-install-and-npm-ci

But npm ci doesn't work w/ our app (it throws a bunch of errors) b/c our package.json and package-lock.json files are out of sync. For example, we have dotenv in package.json, but it's missing from package-lock.json.

yarn install - will give you reproducible builds based off yarn-lock.yaml. After trying so many other things, this is what ended up getting the tests passing for me. *sigh

https://classic.yarnpkg.com/en/docs/yarn-lock

Anywho, from what I'm seeing we either want to use npm or yarn. I think we should pick one and remove the other *-lock.json file. 😅 💆‍♂ 🚶 🌞

@stenington
Copy link
Contributor Author

I agree we should pick one. I believe CI uses yarn so the easiest change is to use that.

shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Update @vue/test-utils
f994ea4 
I upgraded @vue/test-utils to version `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:

 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:

 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Update @vue/test-utils
61151b5 
I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Update @vue/test-utils
4d826f7 
I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380
@shaun-technovation shaun-technovation mentioned this issue Mar 6, 2020
Merged
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Remove package-lock.json
2d39f53 
Using both `package-lock.json` and `yarn-lock.json` can cause confusion
and lead to potential issues, so we'll just use one, `yarn-lock.json`.
:smiley:

Refs: #2380
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Remove 'package-lock.json`
31bcfde 
Using both `package-lock.json` and `yarn-lock.json` can cause confusion
and lead to potential issues, so we'll just use one, `yarn-lock.json`.
:smiley:

Refs: #2380
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Update yarn.lock
16c9ae5 
I ran `yarn update` to get the latest versions our packages, as
specified in `package.json`.

Refs: #2380
shaun-technovation added a commit that referenced this issue Mar 6, 2020
@shaun-technovation
Remove package-lock.json
834dae2 
Using both `package-lock.json` and `yarn-lock.json` can cause confusion
and lead to potential issues, so we'll just use one, `yarn-lock.json`.
:smile:

Refs: #2380
@shaun-technovation
Copy link
Contributor

shaun-technovation commented Mar 6, 2020 •
edited
Loading

I agree we should pick one. I believe CI uses yarn so the easiest change is to use that.

Cool, I removed package-lock.json in #2412

Fwiw, after getting into the weeds on this, I think it would better to use npm for two (pretty much trivial) reasons: it make's the project more accessible/easier for someone to contribute, and removes a dependency. But since we're the only ones who contribute, and CircleCI is already setup to use yarn, then I think it makes sense to just keep it as-is, using yarn. 👍 😀

@stenington
Copy link
Contributor Author

Looks like rails/webpacker has yarn as a prereq, so that may be the reason for the choice here.

shaun-technovation added a commit that referenced this issue Mar 9, 2020
@shaun-technovation
#2380 Update @vue/test-utils (#2412)
c8107cd 
* Update @vue/test-utils

I upgraded @vue/test-utils to the latest version, `v1.0.0-beta.31`.

We were previously on `v1.0.0-beta25`, here's the changelog:
 - https://github.com/vuejs/vue-test-utils/releases

This upgrade broke a bunch of tests; our tests had to refactored
to run asynchronously, here are some links regarding that:
 - vuejs/vue-test-utils#1137
 - https://vue-test-utils.vuejs.org/guides/#writing-asynchronous-tests-using-nexttick-new

Refs: #2380

* Remove package-lock.json

Using both `package-lock.json` and `yarn-lock.json` can cause confusion
and lead to potential issues, so we'll just use one, `yarn-lock.json`.
:smile:

Refs: #2380
@shaun-technovation
Copy link
Contributor

This one is on staging, but there is no front-end piece to this one, so I don't think any manual QAing is necessary. 😃 The CI build is passing, fyi. 👍

@stenington
Copy link
Contributor Author

Yeah, makes sense to me.

@stenington stenington added 7 - Ready to Deploy and removed 6 - Test Keep <= 8 labels Mar 10, 2020
@stenington stenington mentioned this issue Mar 10, 2020
Closed
@rsgonzal rsgonzal removed the 8 - Done label Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shaun-technovation shaun-technovation

Labels
tech debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stenington @hellafitz @rsgonzal @shaun-technovation