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

Update tap to v15 #83

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathansamines
Copy link

@jonathansamines jonathansamines commented Dec 29, 2021

Updated project to tap 15. Most of changes are due to deprecations:

  • Several aliases are now deprecated (i.e t.is, t.throw)
  • --esm option was removed. Using causes the failures seen in Bump tap from 14.11.0 to 15.1.5 #73
  • afterEach no longer supports callbacks

Notes

--coverage=true currently causes some tests to fail. Seems to be related to tapjs/tapjs#669, but I am still investigating. When coverage is enabled, tests that use execSync to verify the log redirection to pino fail because it doesn't happen (debug logs are redirected to stderr instead)

Edit 2

The problem seems to come from the nyc upgrade from v14 to v15:

# fails in tap 15 (nyc 15), but succeeds in tap 14 (nyc 14)
npx tap test/*.js

# fails with nyc 15 but succeeds in nyc 14
npx nyc tap --no-coverage test/*.js

.taprc Show resolved Hide resolved
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm once CI is green

@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2022

Codecov Report

Merging #83 (231198a) into master (a093832) will increase coverage by 2.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #83      +/-   ##
===========================================
+ Coverage   98.00%   100.00%   +2.00%     
===========================================
  Files           2         2              
  Lines          50        50              
===========================================
+ Hits           49        50       +1     
+ Misses          1         0       -1     
Impacted Files Coverage Δ
index.js 100.00% <0.00%> (+2.77%) ⬆️

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 a093832...231198a. Read the comment docs.

@jsumners
Copy link
Member

https://github.com/pinojs/pino-debug/runs/4850023188?check_suite_focus=true#step:6:95 is at least one test that needs to be revised.

@mcollina
Copy link
Member

can you fix CI? It seems it does not work anymore with debug@4

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.

4 participants