Issue #3593, #1197- fix isolate management

test/BaseJsTest.h

Index: test/BaseJsTest.h
diff --git a/test/BaseJsTest.h b/test/BaseJsTest.h
index 48a01f219c4a2718bbb96d367be7d263882c581c..e68aec4a1c7081d2575dd26dd2250f571b5b53f6 100644
--- a/test/BaseJsTest.h
+++ b/test/BaseJsTest.h
@@ -138,6 +138,8 @@ public:
   }
 };
 
+AdblockPlus::JsEnginePtr createJsEngine(const AdblockPlus::AppInfo& appInfo = AdblockPlus::AppInfo());

[Eric, 2016/01/26 14:48:59]

In a suggestion that also appears elsewhere in a different way, the 'appInfo'
argument could be omitted in favor of using application info that identifies the
application as "unit tests".

[Eric, 2016/01/26 14:48:59]

According to our style guide, this should be "CreateJsEngine".

[sergei, 2016/01/27 15:06:10]

How can it be omitted? In some tests we need it in some the default constructed
AppInfo is used and there is no reason to create the second method that calls
JsEngine::New with default AppInfo.

[sergei, 2016/01/27 15:06:11]

Done.

[Eric, 2016/01/27 17:21:02]

OK. I'm not familiar with the unit test suite for this module. It does surprise
me, however, that any tests rely on default 'AppInfo'.

+
 class BaseJsTest : public ::testing::Test
 {
 protected:
@@ -145,11 +147,12 @@ protected:
 
   virtual void SetUp()
   {
-    jsEngine = AdblockPlus::JsEngine::New();
+    jsEngine = createJsEngine();
     jsEngine->SetLogSystem(AdblockPlus::LogSystemPtr(new ThrowingLogSystem));
     jsEngine->SetFileSystem(AdblockPlus::FileSystemPtr(new ThrowingFileSystem));
     jsEngine->SetWebRequest(AdblockPlus::WebRequestPtr(new ThrowingWebRequest));
   }
 };
 
+
 #endif