Skip to content

Commit

Permalink
test: move test-vm-function-redefinition to parallel
Browse files Browse the repository at this point in the history 
With the upstream fix in V8, function declarations now
work fine in the vm module and the test is no longer failing.

Refs: https://codereview.chromium.org/2334733002
Fixes: #548
PR-URL: #9618
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
@fhinkel @targos
fhinkel authored and targos committed Jan 26, 2017
1 parent b37f55a commit c2c6ae5
Showing 1 changed file with 0 additions and 0 deletions.
0 ...n_issues/test-vm-function-redefinition.js → ...parallel/test-vm-function-redefinition.js
View file
File renamed without changes.

5 comments on commit c2c6ae5

@addaleax
Copy link
Member

Choose a reason for hiding this comment

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

@fhinkel fyi, this test fails in debug mode:

Path: parallel/test-vm-function-redefinition
#
# Fatal error in ../deps/v8/src/lookup.h, line 226
# Check failed: has_property_.
#

==== C stack trace ===============================

    out/Debug/node(v8::base::debug::StackTrace::StackTrace()+0x1d) [0x27929ad]
    out/Debug/node(V8_Fatal+0x10b) [0x278f0eb]
    out/Debug/node(v8::internal::LookupIterator::property_details() const+0x3d) [0x166b549]
    out/Debug/node() [0x219c6ca]
    out/Debug/node() [0x219cdd3]
    out/Debug/node() [0x219d2e1]
    out/Debug/node(v8::internal::Runtime_DeclareGlobals(int, v8::internal::Object**, v8::internal::Isolate*)+0x133) [0x219d105]
    [0x31d91a8043a7]
Command: out/Debug/node /home/sqrt/src/node/test/parallel/test-vm-function-redefinition.js

@fhinkel
Copy link
Member Author

Choose a reason for hiding this comment

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

Yikes, OK. Will look into it.

@fhinkel
Copy link
Member Author

@fhinkel fhinkel commented on c2c6ae5 Feb 5, 2017

Choose a reason for hiding this comment

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

Fixed in https://codereview.chromium.org/2675993002/. Should we cherry-pick?

@addaleax
Copy link
Member

Choose a reason for hiding this comment

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

@fhinkel For now the tests will fail in Debug mode anyway due to #7144, so I don’t think it’s very important?

@fhinkel
Copy link
Member Author

@fhinkel fhinkel commented on c2c6ae5 Feb 6, 2017

Choose a reason for hiding this comment

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

Without the fix, we're using a variable without initializing it first. But it's an integer that's cast to an enum. So maybe not really dangerous?

Please sign in to comment.