Closed mmarchini closed 4 years ago
I'm getting the OS X failures on master as well, and we're also seeing it on the daily runs: https://github.com/nodejs/llnode/runs/603630513?check_suite_focus=true. I think something changed on OS X lldb output when running a program, because that's where the test is failing. Will open an issue to investigate. In the meantime, llnode should still work on OS X, but we don't have coverage until the issue is resolved.
Landed in dd57bfb8af3c
We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report. To ensure accuracy in future PRs, please see these guidelines. A quick fix for this PR: rebase it; your next report should be accurate.
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
src/llv8.cc | 3 | 6 | 50.0% | ||
src/llv8-inl.h | 18 | 24 | 75.0% | ||
<!-- | Total: | 29 | 38 | 76.32% | --> |
Totals | |
---|---|
Change from base Build fdddce0d2c7d15993584ec828569b94d61ac374a: | 4.1% |
Covered Lines: | 3758 |
Relevant Lines: | 4743 |
V8 8.1 disabled unboxed double fieds, which means all double fields are now stored on memory as references to HeapNumber objects instead of the double value. In theory V8 could store boxed doubles before, but we didn't take it into account in our dobule fields code. In the future, V8 intends to re-enable unboxed doubles, which means we'll need to add logic to handle both types of fields in the same llnode session.
This also fixes a bug where llnode was not handling double fields at all in some cases.
Fixes: https://github.com/nodejs/llnode/issues/353 Signed-off-by: Matheus Marchini mmarchini@netflix.com