test: using v8::Context::Scope in cctest EnvironmentTestFixture

Fixes nodejs#529 PR-URL: nodejs#532 Reviewed-By: Taylor Woll <tawoll@ntdev.microsoft.com>
kfarnung · May 10, 2018 · 2cd0485 · 2cd0485
1 parent 68c92c1
commit 2cd0485
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 12 deletions.
diff --git a/deps/chakrashim/src/v8handlescope.cc b/deps/chakrashim/src/v8handlescope.cc
@@ -31,6 +31,7 @@ HandleScope::HandleScope(Isolate* isolate)
       _count(0),
       _contextRef(JS_INVALID_REFERENCE),
       _addRefRecordHead(nullptr) {
+  CHAKRA_ASSERT(isolate == Isolate::GetCurrent());
   _locals[0] = JS_INVALID_REFERENCE;
   current = this;
 }

diff --git a/node.gyp b/node.gyp
@@ -1037,10 +1037,6 @@
           'include_dirs': [
             'deps/chakrashim/include'
           ],
-          'sources!': [
-            'test/cctest/test_environment.cc', # TODO: Enable these test for node-chakracore
-            'test/cctest/test_node_postmortem_metadata.cc',
-          ],
           'conditions': [
             [ 'OS!="win" and chakracore_use_lto=="true"', {
               'ldflags': [

diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h
@@ -87,9 +87,11 @@ class NodeTestFixture : public ::testing::Test {
   virtual void SetUp() {
     isolate_ = v8::Isolate::New(params);
     CHECK_NE(isolate_, nullptr);
+    isolate_->Enter();
   }
 
   virtual void TearDown() {
+    isolate_->Exit();
     isolate_->Dispose();
     isolate_ = nullptr;
   }
@@ -100,16 +102,16 @@ class EnvironmentTestFixture : public NodeTestFixture {
  public:
   class Env {
    public:
-    Env(const v8::HandleScope& handle_scope, const Argv& argv) {
-      auto isolate = handle_scope.GetIsolate();
+    Env(const v8::HandleScope& handle_scope, const Argv& argv) :
 #if ENABLE_TTD_NODE
-      // TODO(MSLaguana): should we support TTD in cctest?
-      context_ = node::NewContext(isolate, false);
+        // TODO(MSLaguana): should we support TTD in cctest?
+        context_(node::NewContext(handle_scope.GetIsolate(), false)),
 #else
-      context_ = node::NewContext(isolate);
+        context_(node::NewContext(handle_scope.GetIsolate())),
 #endif
+        context_scope_(context_) {
+      auto isolate = handle_scope.GetIsolate();
       CHECK(!context_.IsEmpty());
-      context_->Enter();
 
       isolate_data_ = node::CreateIsolateData(isolate,
                                               &NodeTestFixture::current_loop,
@@ -125,7 +127,6 @@ class EnvironmentTestFixture : public NodeTestFixture {
     ~Env() {
       node::FreeEnvironment(environment_);
       node::FreeIsolateData(isolate_data_);
-      context_->Exit();
     }
 
     node::Environment* operator*() const {
@@ -136,8 +137,10 @@ class EnvironmentTestFixture : public NodeTestFixture {
       return context_;
     }
 
+
    private:
     v8::Local<v8::Context> context_;
+    v8::Context::Scope context_scope_;
     node::IsolateData* isolate_data_;
     node::Environment* environment_;
     DISALLOW_COPY_AND_ASSIGN(Env);

diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc
@@ -38,7 +38,6 @@ class RepostingTask : public v8::Task {
 class PlatformTest : public EnvironmentTestFixture {};
 
 TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) {
-  v8::Isolate::Scope isolate_scope(isolate_);
   const v8::HandleScope handle_scope(isolate_);
   const Argv argv;
   Env env {handle_scope, argv};