fixed ioIndexedDB tests

qunit/tests/ioIndexedDB.js

Index: qunit/tests/ioIndexedDB.js
diff --git a/qunit/tests/ioIndexedDB.js b/qunit/tests/ioIndexedDB.js
index 58defadc4d6e9624370d50d85f7c80067e5f65fb..40edb5d0007c47d5aa6b78795b0847b61d20a27d 100644
--- a/qunit/tests/ioIndexedDB.js
+++ b/qunit/tests/ioIndexedDB.js
@@ -2,6 +2,8 @@
 
 {
   const {IO} = require("io");
+  const info = require("info");
+
 
   const testFileNames = {
     testData: "testData",
@@ -16,75 +18,95 @@
     lastModified: Date.now()
   };
 
-  QUnit.module("IO tests", {
+  let isEdge = info.platform == "edgehtml";
+  let testEdge = isEdge ? QUnit.test : QUnit.skip;
+
+  QUnit.module("Edge filter storage", {
     beforeEach()
     {
-      return prePopulateStorage();
+      if (isEdge)

[kzar, 2018/07/05 09:54:15]

I don't think this check (and the one in afterEach) is necessary since the tests
are skipped anyway.

[piscoi.georgiana, 2018/07/09 14:04:31]

You are right, I don't know why I was under the impression that those 2
functions are called. Either way, I've removed the checks.

[kzar, 2018/07/09 14:17:28]

Acknowledged.

+        return prePopulateStorage();
     },
     afterEach()
     {
-      return clearStorage();
+      if (isEdge)
+        return clearStorage();
     }
   });
 
-  test("statFile", assert =>
+  function prePopulateStorage()
   {
-    const noFileMsg = "returns correct value if file doesn't exist";
-    const fileExistsMsg = "returns correct value if file exists";
-
-    ok(IO.statFile(testFileNames.simpleCheck) instanceof Promise,
-      "returns a promise");
+    return new Promise(resolve =>
+    {
+      let db;
+      let req = indexedDB.open("adblockplus", 1);
 
-    asyncReadHelper(
-      IO.statFile,
-      testFileNames.testData,
-      {exists: true, lastModified: testData.lastModified},
-      fileExistsMsg,
-      assert);
+      req.onsuccess = event =>
+      {
+        db = event.currentTarget.result;
+        let store = db
+          .transaction(["file"], "readwrite")
+          .objectStore("file");
 
-    asyncReadHelper(
-      IO.statFile,
-      testFileNames.simpleCheck,
-      {exists: false},
-      noFileMsg,
-      assert);
-  });
+        store.put(testData).onsuccess = () => resolve();
+      };
+    });
+  }
 
-  test("writeToFile", assert =>

[kzar, 2018/07/05 09:54:15]

Seems like a bunch of unrelated changes here? I'd prefer it if you just changed
(more or less) what was necessary for the task at hand. That would make it
easier to review.

[piscoi.georgiana, 2018/07/09 14:04:31]

Sorry about that. In the future I'll leave this sort of changes for last, but I
still want them in, as the intention was to improve code locality.

[kzar, 2018/07/09 14:17:28]

Sure, no problem. FWIW I agree that it's good to improve code locality where
possible.

Would you mind removing those unrelated changes from this code review though? I
think they belong in a separate review (and commit). Don't worry it doesn't
require filing another issue, for more trivial changes like those we label the
commit "Noissue - ...".

[piscoi.georgiana, 2018/07/09 16:38:49]

Done.

+  function clearStorage()
   {
-    ok(IO.writeToFile(testFileNames.simpleCheck, ["test"]) instanceof Promise,
-      "returns a promise");
+    return new Promise(resolve =>
+    {
+      let db;
+      let req = indexedDB.open("adblockplus", 1);
 
-    writesCorrectValue(assert);
-  });
+      req.onsuccess = event =>
+      {
+        db = event.currentTarget.result;
+        let files = Object.values(testFileNames)
+          .map(fileName => new Promise((resolveFile, reject) =>
+          {
+            let store = db
+              .transaction(["file"], "readwrite")
+              .objectStore("file");
 
-  function writesCorrectValue(assert)
+            store.delete("file:" + fileName).onsuccess = resolveFile;
+          }));
+
+        Promise.all(files).then(resolve);
+      };
+    });
+  }
+
+  testEdge("statFile", assert => testStatFile(assert));
+  testEdge("readFromFile", assert => testReadFromFile(assert));
+  testEdge("writeToFile", assert => testWriteToFile(assert));
+  testEdge("renameFile", assert => testRenameFile(assert));
+
+  function testStatFile(assert)
   {
-    const writeCheck = {
-      fileName: "file:writeCheck",
-      content: [1, 2, 3],
-      lastModified: Date.now()
-    };
-    let done = assert.async();
+    const noFileMsg = "returns correct value if file doesn't exist";
+    const fileExistsMsg = "returns correct value if file exists";
 
-    IO.writeToFile(testFileNames.write, writeCheck.content)
-      .then(() => readFromStorage(writeCheck.fileName))
-      .then(result =>
-      {
-        deepEqual(
-          Object.keys(writeCheck),
-          Object.keys(result),
-          "data is written in the correct format");
+    ok(IO.statFile(testFileNames.simpleCheck) instanceof Promise,
+        "returns a promise");
 
-        deepEqual(
-          writeCheck.content,
-          result.content,
-          "data has the correct content");
-        done();
-      });
+    asyncReadHelper(
+        IO.statFile,
+        testFileNames.testData,
+        {exists: true, lastModified: testData.lastModified},
+        fileExistsMsg,
+        assert);
+
+    asyncReadHelper(
+        IO.statFile,
+        testFileNames.simpleCheck,
+        {exists: false},
+        noFileMsg,
+        assert);
   }
 
-  test("readFromFile", assert =>
+  function testReadFromFile(assert)
   {
     const noFileMsg = "returns correct value if file doesn't exist";
 
@@ -100,14 +122,31 @@
     );
 
     callsListeners(assert);
-  });
+  }
+
+  function asyncReadHelper(method, fileName, expectedValue, description, assert)
+  {
+    let done = assert.async();
+
+    method(fileName)
+    .then(result =>
+    {
+      deepEqual(result, expectedValue, description);
+      done();
+    })
+    .catch(error =>
+    {
+      deepEqual(error, expectedValue, description);
+      done();
+    });
+  }
 
   function callsListeners(assert)
   {
     let done = assert.async();
     let called = [];
 
-    IO.readFromFile(testFileNames.testData, (entry) => called.push(entry))
+    IO.readFromFile(testFileNames.testData, entry => called.push(entry))
       .then(() =>
       {
         deepEqual(
@@ -118,13 +157,47 @@
       });
   }
 
-  test("renameFile", assert =>
+  function testWriteToFile(assert)
+  {
+    ok(IO.writeToFile(testFileNames.simpleCheck, ["test"]) instanceof Promise,
+    "returns a promise");
+
+    writesCorrectValue(assert);
+  }
+
+  function writesCorrectValue(assert)
+  {
+    const writeCheck = {
+      fileName: "file:writeCheck",
+      content: [1, 2, 3],
+      lastModified: Date.now()
+    };
+    let done = assert.async();
+
+    IO.writeToFile(testFileNames.write, writeCheck.content)
+      .then(() => readFromStorage(writeCheck.fileName))
+      .then(result =>
+      {
+        deepEqual(
+          Object.keys(writeCheck),
+          Object.keys(result),
+          "data is written in the correct format");
+
+        deepEqual(
+          writeCheck.content,
+          result.content,
+          "data has the correct content");
+        done();
+      });
+  }
+
+  function testRenameFile(assert)
   {
     ok(IO.renameFile(testFileNames.simpleCheck) instanceof Promise,
-      "returns a promise");
+    "returns a promise");
 
     checkRename(assert);
-  });
+  }
 
   function checkRename(assert)
   {
@@ -144,29 +217,13 @@
       });
   }
 
-  function asyncReadHelper(method, fileName, expectedValue, description, assert)
-  {
-    let done = assert.async();
-    method(fileName)
-    .then(result =>
-    {
-      deepEqual(result, expectedValue, description);
-      done();
-    })
-    .catch(error =>
-    {
-      deepEqual(error, expectedValue, description);
-      done();
-    });
-  }
-
   function readFromStorage(fileName)
   {
     return new Promise(resolve =>
     {
       let db;
       let req = indexedDB.open("adblockplus", 1);
-      req.onsuccess = (event) =>
+      req.onsuccess = event =>
       {
         db = event.currentTarget.result;
         let store = db
@@ -179,49 +236,4 @@
       };
     });
   }
-
-  function prePopulateStorage()
-  {
-    return new Promise(resolve =>
-    {
-      let db;
-      let req = indexedDB.open("adblockplus", 1);
-
-      req.onsuccess = (event) =>
-      {
-        db = event.currentTarget.result;
-        let store = db
-          .transaction(["file"], "readwrite")
-          .objectStore("file");
-
-        store.put(testData).onsuccess = resolve;
-      };
-    });
-  }
-
-  function clearStorage()
-  {
-    return new Promise(resolve =>
-      {
-      let db;
-      let req = indexedDB.open("adblockplus", 1);
-
-      req.onsuccess = (event) =>
-      {
-        db = event.currentTarget.result;
-        let files = Object.keys(testFileNames)
-          .map(fileName => new Promise((resolveFile, reject) =>
-          {
-            let store = db
-              .transaction(["file"], "readwrite")
-              .objectStore("file");
-
-            store.delete("file:" + fileName).onsuccess = resolveFile;
-          }));
-
-        Promise.all(files).then(resolve);
-      };
-    });
-  }
 }
-