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

test: additional tests for relative path support #490

Merged
merged 2 commits into from
Jan 15, 2024

Conversation

mcknasty
Copy link
Contributor

@mcknasty mcknasty commented Jul 30, 2023

Pull Request 490

Artifact Directory Unit Tests

test: relative directories for tmpDir and reportsDir
test: absolute directories for tmpDir and reportsDir

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. 2 failing
    3. 97 passing in 50 seconds
  6. Now update the snapshot.
    1. A complete log of snapshot test results.
    2. 99 passing in 46 seconds
  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. 99 passing in 50 seconds
  10. Push
  11. Open the pull request, see details in the template.
  12. Make any necessary changes after review.

Unit Tests Results

Test Type PASS Tests Passed Tests Failed Time
Initial Test 97 passing 2 failing 50 seconds
Snapshot Test ✔️ 99 passing 0 failures 46 seconds
Final Test ✔️ 99 passing 0 failures 50 seconds

Node Version Testing Matrix

Node Version PASS Tests Passed Tests Failed Time
10.24.1*
12.22.12 ✔️ 99 passing 0 failures 1 minutes
14.21.3 ✔️ 99 passing 0 failures 1 minutes
16.20.1 ✔️ 99 passing 0 failures 59 seconds
18.17.0 ✔️ 99 passing 0 failures 54 seconds

*Node version 10.x support has been deprecated.

@mcknasty mcknasty marked this pull request as draft July 30, 2023 17:23
@mcknasty
Copy link
Contributor Author

Leaving a note to work on variations of the 4 new unit tests including utilizing the NODE_V8_COVERAGE shell variable.

#447
#448
#489

test: relative directories for tmpDir and reportsDir
test: absolute directories for tmpDir and reportsDir
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.

Does this increase coverage should we land it?

@mcknasty mcknasty marked this pull request as ready for review January 15, 2024 00:17
@mcknasty
Copy link
Contributor Author

@bcoe They look about the same. There is a small rounding difference but the uncovered lines are the same. Additional tests don't hurt though, especially when it comes to Test Driven Development.

HEAD of main branch


95 passing (50s)

--------------------------|---------|----------|---------|---------|----------------------------------
File                      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------------|---------|----------|---------|---------|----------------------------------
All files                 |   97.16 |     88.8 |     100 |   97.16 |
 bin                      |   95.34 |      100 |     100 |   95.34 |
  c8.js                   |   95.34 |      100 |     100 |   95.34 | 41-42
 lib                      |   96.83 |    86.31 |     100 |   96.83 |
  parse-args.js           |   98.21 |       75 |     100 |   98.21 | 165-166,201-202
  report.js               |   97.51 |     91.3 |     100 |   97.51 | 9-10,121,233-234,317-318,352-354
  source-map-from-file.js |      91 |    71.42 |     100 |      91 | 74-76,91-93,95-97
 lib/commands             |     100 |    95.45 |     100 |     100 |
  check-coverage.js       |     100 |    92.85 |     100 |     100 | 26
  report.js               |     100 |      100 |     100 |     100 |
--------------------------|---------|----------|---------|---------|----------------------------------

This patch


99 passing (42s)

--------------------------|---------|----------|---------|---------|----------------------------------
File                      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------------|---------|----------|---------|---------|----------------------------------
All files                 |   97.16 |     88.7 |     100 |   97.16 |
 bin                      |   95.34 |      100 |     100 |   95.34 |
  c8.js                   |   95.34 |      100 |     100 |   95.34 | 41-42
 lib                      |   96.83 |    86.17 |     100 |   96.83 |
  parse-args.js           |   98.21 |       75 |     100 |   98.21 | 165-166,201-202
  report.js               |   97.51 |    91.17 |     100 |   97.51 | 9-10,121,233-234,317-318,352-354
  source-map-from-file.js |      91 |    71.42 |     100 |      91 | 74-76,91-93,95-97
 lib/commands             |     100 |    95.45 |     100 |     100 |
  check-coverage.js       |     100 |    92.85 |     100 |     100 | 26
  report.js               |     100 |      100 |     100 |     100 |
--------------------------|---------|----------|---------|---------|----------------------------------

@bcoe bcoe changed the title Artifact Directory Unit Tests test: additional tests for relative path support Jan 15, 2024
@bcoe bcoe merged commit bf3073b into bcoe:main Jan 15, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants