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

Improve code quality by increasing code coverage: lib/source-map-from-file.js #453

Closed

Conversation

mcknasty
Copy link
Contributor

@mcknasty mcknasty commented Feb 10, 2023

Pull Request 453

commit ff5d39b73f2265105a8ecf1cc4b5fd9f47a570e2
Author: The Nasty [email protected]
Date: Tue Jan 16 14:00:38 2024 -0500

Improve code quality by increasing code coverage: lib/source-map-from-file.js (#453)

refactor:  lib/source-map-from-file.js to improve code coverage (#453)
refactor: exposed source-map-from-file.js function to write test cases (#453)
test: Added two test case cases to cover error handling for improper formatting (#453)

Following the Contributions Recommendations here.

  1. Make sure you have installed the latest version of Node.js
  2. Fork this project on GitHub and clone your fork locally
  3. Create local branches to work within. These should also be created directly from the main branch. Local Fork here.
  4. Make your changes.
  5. Run tests to make sure all is okay (everything should pass except the snapshot).
    1. A complete log of initial test results.
    2. As instructed, ignore snapshot failures. 0 failures
    3. 101 passing in 1 minutes
  6. Now update the snapshot.
    1. A complete log of snapshot test results.
    2. 101 passing in 1 minutes
  7. If all is passing, commit your changes. The log of the commit can be found here.
  8. As a best practice, once you have committed your changes, it is a good idea to use git rebase (not git merge) to synchronize your work with the main repository.
  9. Run tests again to make sure all is okay.
    1. A complete log of the final test results.
    2. 101 passing in 49 seconds
  10. Push
  11. Open the pull request, see details in the template.
  12. Make any necessary changes after review.

File Code Coverage Matrix Report

Test Type File Statement Branch Function Lines PASS
Final Test lib/source-map-from-file.js 100% 93.33% 100% 100%
Node 14.21.3 lib/source-map-from-file.js 100% 93.33% 100% 100%
Node 16.20.2 lib/source-map-from-file.js 100% 93.33% 100% 100%
Node 18.19.0 lib/source-map-from-file.js 100% 93.33% 100% 100%
Node 20.11.0 lib/source-map-from-file.js 100% 93.33% 100% 100%

Unit Tests Results

Test Type PASS Tests Passed Tests Failed Time
Initial Test ✔️ 101 passing 0 failures 1 minutes
Snapshot Test ✔️ 101 passing 0 failures 1 minutes
Final Test ✔️ 101 passing 0 failures 49 seconds
  • Tests conducted with Node v18.19.0

Node Version Testing Matrix

Node Version PASS Tests Passed Tests Failed Time
14.21.3 ✔️ 101 passing 0 failures 1 minutes
16.20.2 ✔️ 101 passing 0 failures 58 seconds
18.19.0 ✔️ 101 passing 0 failures 60 seconds
20.11.0 ✔️ 101 passing 0 failures 59 seconds

References #448

  • Updated 2024-01-16 4:39 PM EST

mcknasty added a commit to mcknasty/c8 that referenced this pull request Feb 10, 2023
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formating (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 78378b4 to 67ad118 Compare February 10, 2023 00:37
mcknasty added a commit to mcknasty/c8 that referenced this pull request Feb 10, 2023
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formating (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 67ad118 to 6ae87f2 Compare February 10, 2023 02:46
lib/source-map-from-file.js Outdated Show resolved Hide resolved
Copy link
Owner

@bcoe bcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good to me, the one thing that seems a bit weird is the if (nodeMajorVersion === 12) because I don't think the logic that skips lines is smart enough to take if statements into account.

lib/source-map-from-file.js Outdated Show resolved Hide resolved
test/source-map-from-file.js Show resolved Hide resolved
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jul 29, 2023
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formating (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 6ae87f2 to 6adda9f Compare July 29, 2023 18:17
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jul 29, 2023
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 6adda9f to 433aefa Compare July 29, 2023 20:15
@mcknasty mcknasty requested a review from bcoe July 29, 2023 20:21
@mcknasty mcknasty marked this pull request as ready for review July 29, 2023 20:23
@bcoe bcoe added the ci label Jan 2, 2024
@bcoe
Copy link
Owner

bcoe commented Jan 3, 2024

This looks ready to land to me, apologies for ignoring for so long.

Mind rebasing?

mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 3, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch 2 times, most recently from df725fa to 708e64a Compare January 3, 2024 21:23
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 3, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty
Copy link
Contributor Author

mcknasty commented Jan 3, 2024

@bcoe ready to go!

mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 4, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 708e64a to 95194b7 Compare January 4, 2024 23:03
@mcknasty
Copy link
Contributor Author

mcknasty commented Jan 4, 2024

Completed another rebase after release.

@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 95194b7 to e967873 Compare January 4, 2024 23:06
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 4, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty
Copy link
Contributor Author

mcknasty commented Jan 5, 2024

Everything looks good except the tests on node 14. The CI process fails with the following error:

Run npm ci --engine-strict
npm ERR! Cannot read property '@bcoe/v8-coverage' of undefined

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/202[4](https://github.com/bcoe/c8/actions/runs/7415825716/job/20179695732?pr=453#step:4:5)-01-04T23_0[7](https://github.com/bcoe/c8/actions/runs/7415825716/job/20179695732?pr=453#step:4:8)_00_[8](https://github.com/bcoe/c8/actions/runs/7415825716/job/20179695732?pr=453#step:4:9)93Z-debug.log
Error: Process completed with exit code 1.

If I run the same command locally on Node 14, I get the following:

❯ sh
$ node -v
v14.21.3
$ npm ci --engine-strict

added 384 packages, and audited 385 packages in 7s

99 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
$ uname -a
Linux XXXX-XXXXXX 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
$

I don't think this is anything on my end. @bcoe any ideas?

mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 14, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from e967873 to 7118a2e Compare January 14, 2024 22:48
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 14, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 7118a2e to c928670 Compare January 14, 2024 22:53
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 16, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch 2 times, most recently from ff5d39b to 60c7925 Compare January 20, 2024 10:29
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 20, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage (bcoe#453)
refactor: exposed source-map-from-file.js function to write test cases (bcoe#453)
test: Added two test case cases to cover error handling for improper formatting (bcoe#453)
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 60c7925 to 856991a Compare January 20, 2024 10:53
@mcknasty
Copy link
Contributor Author

I am going to pull this back into draft mode. I noticed another module for spies that looks much easier to implement.

@mcknasty mcknasty marked this pull request as draft January 20, 2024 20:23
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 856991a to ff5d39b Compare January 20, 2024 20:24
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 20, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from ff5d39b to 8e093d8 Compare January 20, 2024 20:28
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jan 20, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from 8e093d8 to ac37487 Compare January 20, 2024 20:54
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
@mcknasty mcknasty force-pushed the lib-source-map-from-file-code-coverage branch from ac37487 to fd55f06 Compare January 20, 2024 21:01
@bcoe bcoe closed this Jun 9, 2024
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jul 17, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
mcknasty added a commit to mcknasty/c8 that referenced this pull request Jul 17, 2024
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants