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

chore(ci): temporarily disable OS X #368

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

mmarchini
Copy link
Contributor

Refs: #362

@mmarchini
Copy link
Contributor Author

cc @nodejs/llnode

@joyeecheung
Copy link
Member

It seems that --abort-on-uncaught-exception has regressed on Node.js v14 which is why the test is timing out (instead of aborting, the process exits with -1 normally)

joyeecheung
joyeecheung approved these changes Aug 6, 2020
.github/workflows/push.yml Show resolved Hide resolved
@mmarchini
Copy link
Contributor Author

It seems that --abort-on-uncaught-exception has regressed on Node.js v14 which is why the test is timing out (instead of aborting, the process exits with -1 normally)

Ah yes, that might cause v14 tests to fail, although I'm pretty sure llnode is broken on v14 since we bumped V8 recently (as in, after the last time we updated things on llnode).

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2020

Codecov Report

Merging #368 into master will increase coverage by 0.05%.
The diff coverage is 81.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   78.93%   78.98%   +0.05%     
==========================================
  Files          33       33              
  Lines        4262     4744     +482     
==========================================
+ Hits         3364     3747     +383     
- Misses        898      997      +99     
Impacted Files Coverage Δ
src/constants.h 88.88% <ø> (+11.11%) ⬆️
src/error.cc 82.60% <0.00%> (-1.61%) ⬇️
src/error.h 100.00% <ø> (+12.50%) ⬆️
src/llv8.cc 71.71% <56.75%> (-1.35%) ⬇️
src/printer.cc 81.15% <68.33%> (+3.45%) ⬆️
src/llv8-inl.h 90.42% <84.37%> (-2.50%) ⬇️
test/plugin/frame-test.js 83.33% <84.61%> (-4.17%) ⬇️
src/llv8-constants.cc 85.92% <91.66%> (+2.91%) ⬆️
src/constants.cc 82.08% <100.00%> (+7.84%) ⬆️
src/llscan.cc 59.41% <100.00%> (-1.85%) ⬇️
... and 26 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 fe9a668...e2906bd. Read the comment docs.

@joyeecheung
Copy link
Member

From nodejs/node#32981 it seems the bug was introduced from v12.11.0. Is there any progress in the V8 upstream?

@mmarchini
Copy link
Contributor Author

I don't think I opened an upstream bug 🤔 I'll check tomorrow and will open one if I haven't yet.

@mmarchini
Copy link
Contributor Author

Although tests are passing on v12, so that issue should be unrelated (hopefully)

@mmarchini mmarchini merged commit b69b744 into nodejs:master Aug 6, 2020
@mmarchini mmarchini deleted the disable-osx-tests branch August 6, 2020 06:17
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.

3 participants