Skip to content

feat(testing): add test for src/node/constants.ts #3290

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
Merged
merged 2 commits into from
May 6, 2021
Merged

feat(testing): add test for src/node/constants.ts #3290

merged 2 commits into from
May 6, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented May 4, 2021 •
edited
Loading

This PR adds additional testing for src/node/constants.ts, specifically checking that both commit and version return the correct values when defined in and missing from package.json.

  • update coveragePathIgnorePatterns to /out (so that we collect coverage for src/node/routes
  • add tests for src/node/constants.ts

Screenshot

image

@jsjoeio jsjoeio force-pushed the jsjoeio/update-constants-test branch from f6b2be2 to bcb98a6 Compare May 5, 2021 22:33
@jsjoeio jsjoeio force-pushed the jsjoeio/update-constants-test branch from bcb98a6 to 027106a Compare May 5, 2021 22:34
@jsjoeio jsjoeio self-assigned this May 5, 2021
@jsjoeio jsjoeio added the testing Anything related to testing label May 5, 2021
@jsjoeio jsjoeio added this to the v3.9.4 milestone May 5, 2021
@codecov
Copy link

codecov bot commented May 5, 2021 •
edited
Loading

Codecov Report

Merging #3290 (7f41e45) into main (af5a1c9) will decrease coverage by 0.58%.
The diff coverage is n/a.

❗ Current head 7f41e45 differs from pull request most recent head cb5ab48. Consider uploading reports for the commit cb5ab48 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3290      +/-   ##
==========================================
- Coverage   57.54%   56.95%   -0.59%     
==========================================
  Files          24       35      +11     
  Lines        1279     1703     +424     
  Branches      290      374      +84     
==========================================
+ Hits          736      970     +234     
- Misses        441      583     +142     
- Partials      102      150      +48
Impacted Files Coverage Δ
src/node/routes/health.ts 100.00% <0.00%> (ø)
src/node/routes/domainProxy.ts 64.10% <0.00%> (ø)
src/node/routes/update.ts 77.77% <0.00%> (ø)
src/node/routes/index.ts 67.61% <0.00%> (ø)
src/node/routes/pathProxy.ts 67.85% <0.00%> (ø)
src/node/routes/login.ts 45.28% <0.00%> (ø)
src/node/routes/apps.ts 100.00% <0.00%> (ø)
src/node/routes/logout.ts 50.00% <0.00%> (ø)
src/node/routes/static.ts 30.23% <0.00%> (ø)
src/node/routes/vscode.ts 30.10% <0.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af5a1c9...cb5ab48. Read the comment docs.

@jsjoeio jsjoeio marked this pull request as ready for review May 5, 2021 22:44
@jsjoeio jsjoeio requested a review from a team as a code owner May 5, 2021 22:44
@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 5, 2021 •
edited
Loading

Codecov: "Merging #3290 (027106a) into main (af5a1c9) will not change coverage."

Me:
Alt Text

code-asher
code-asher approved these changes May 6, 2021
View reviewed changes
@jsjoeio jsjoeio force-pushed the jsjoeio/update-constants-test branch from 7f41e45 to 7870c0b Compare May 6, 2021 17:37
code-asher
code-asher reviewed May 6, 2021
View reviewed changes
@jsjoeio
fix: coveragePathIgnorePatterns to /out
cb5ab48 
We were accidentally ignoring `node/routes` because we had "out"
instead of "/out" in `coveragePathIgnorePatterns` which caused
us to not collect coverage for those files. Now we do.
@jsjoeio jsjoeio force-pushed the jsjoeio/update-constants-test branch from c8ed585 to cb5ab48 Compare May 6, 2021 18:53
@jsjoeio jsjoeio merged commit 3243bb3 into main May 6, 2021
@jsjoeio jsjoeio deleted the jsjoeio/update-constants-test branch May 6, 2021 19:25
@jsjoeio jsjoeio linked an issue May 14, 2021 that may be closed by this pull request
[dev]: update node/constants.ts test to cover L19 #3289
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@code-asher code-asher code-asher approved these changes

Assignees

@jsjoeio jsjoeio

Labels
testing Anything related to testing
Projects
None yet
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

[dev]: update node/constants.ts test to cover L19
2 participants
@jsjoeio @code-asher