-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: test-tick-processor was failing on mac osx. #6091
Conversation
Test was looking for "RunInDebugContext", but output had "runInDebugContext". Updated pattern to use lower-case r and test passes.
Both strings (
For
/cc @nodejs/v8 @matthewloring for comments on which string matters for this test. |
This looks like a duplicate of #5903/#5911. Does @indutny 's patch #5903 (comment) fix things? |
@@ -35,7 +35,7 @@ if (common.isWindows || | |||
console.log('1..0 # Skipped: C++ symbols are not mapped for this os.'); | |||
return; | |||
} | |||
runTest(/RunInDebugContext/, | |||
runTest(/runInDebugContext/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/[rR]unInDebugContext/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but this is not a fix for the problem. Something like #5903 (comment) should be done to fix, I'm working with V8 authorities to resolve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
Great. Thanks all. Questions: Is there an ETA on getting the right v8 fix in? And what should be done in the interim given there's a false-failure in the tests? |
@mike-kaufman Right now, it is a matter of landing https://codereview.chromium.org/1840633002/ in V8 and cherry-picking it in node.js . From what I just heard from Michael Achenbach, it looks like |
OK, I'll close this and wait for the v8 fix. |
Checklist
Affected core subsystem(s)
test
Description of change
Test was looking for "RunInDebugContext", but output had
"runInDebugContext". Updated pattern to use lower-case r and test passes.