Issue 4223 - Adapt filter storage read/write tests to work in adblockpluscore repository

test/filterStorage_readwrite.js

-(function()
-{
-  module("Filter storage read/write", {
-    setup: function()
-    {
-      prepareFilterComponents.call(this);
-      preparePrefs.call(this);
-
-      FilterStorage.addSubscription(Subscription.fromURL("~fl~"));
-    },
-    teardown: function()
-    {
-      restoreFilterComponents.call(this);
-      restorePrefs.call(this);
-    }
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2016 Eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+"use strict";
+
+let {createSandbox, unexpectedError} = require("./_common");
+
+let Filter = null;
+let FilterNotifier = null;
+let FilterStorage = null;
+let IO = null;
+let Prefs = null;
+let Subscription = null;
+let ExternalSubscription = null;
+
+exports.setUp = function(callback)
+{
+  let sandboxedRequire = createSandbox();
+  (
+    {Filter} = sandboxedRequire("../lib/filterClasses"),
+    {FilterNotifier} = sandboxedRequire("../lib/filterNotifier"),
+    {FilterStorage} = sandboxedRequire("../lib/filterStorage"),
+    {IO} = sandboxedRequire("./stub-modules/io"),
+    {Prefs} = sandboxedRequire("./stub-modules/prefs"),
+    {Subscription, ExternalSubscription} = sandboxedRequire("../lib/subscriptionClasses")
+  );
+
+  FilterStorage.addSubscription(Subscription.fromURL("~fl~"));
+  callback();
+}
+
+let testData = new Promise((resolve, reject) =>
+{
+  let fs = require("fs");
+  let path = require("path");
+  let datapath = path.resolve(__dirname, "data", "patterns.ini");

[kzar, 2016/10/06 05:13:38]

Did you forget to add the data/patterns.ini file?

[Wladimir Palant, 2016/10/06 07:13:59]

No, I copied the files in a separate commit - what you have here is merely a
diff from the original files. The patterns.ini we use for tests wasn't modified.

[kzar, 2016/10/06 08:06:47]

Acknowledged.

+
+  fs.readFile(datapath, "utf-8", (error, data) =>
+  {
+    if (error)
+      reject(error);
+    else
+      resolve(data);
   });
-
-  let {FileUtils} = Cu.import("resource://gre/modules/FileUtils.jsm", null);
-  let {NetUtil} = Cu.import("resource://gre/modules/NetUtil.jsm", null);
-
-  function loadFilters(file, callback)
+});
+
+function loadFilters(file)
+{
+  return new Promise((resolve, reject) =>
   {
     let listener = function(action)

[kzar, 2016/10/06 05:13:38]

Seems like this logic is nearly identical in saveFilters, perhaps reuse it?

[Wladimir Palant, 2016/10/06 07:13:59]

Reusing isn't quite trivial but with the new FilterNotifier API this is no
longer an issue.

     {
       if (action == "load")
       {
         FilterNotifier.removeListener(listener);
-        callback();
+        resolve();
       }
     };
     FilterNotifier.addListener(listener);
 
     FilterStorage.loadFromDisk(file);
-  }
-
-  function writeToFile(file, data)
-  {
-    let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].createInstance(Ci.nsIScriptableUnicodeConverter);
-    converter.charset = "utf-8";
-    data = converter.ConvertFromUnicode(data);
-
-    let stream = FileUtils.openFileOutputStream(file);
-    stream.write(data, data.length);
-    stream.close();
-  }
-
-  function saveFilters(file, callback)
+  });
+}
+
+function saveFilters(file, callback)
+{
+  return new Promise((resolve, reject) =>
   {
     let listener = function(action)
     {
       if (action == "save")
       {
         FilterNotifier.removeListener(listener);
-        callback();
+        resolve();
       }
     };
     FilterNotifier.addListener(listener);
 
     FilterStorage.saveToDisk(file);
+  });
+}
+
+function testReadWrite(test, withExternal)
+{
+  let tempFile = IO.resolveFilePath("temp_patterns1.ini");
+  let tempFile2 = IO.resolveFilePath("temp_patterns2.ini");
+
+  function canonize(data)
+  {
+    let curSection = null;
+    let sections = [];
+    for (let line of (data + "\n[end]").split(/[\r\n]+/))
+    {
+      if (/^\[.*\]$/.test(line))
+      {
+        if (curSection)
+          sections.push(curSection);
+
+        curSection = {header: line, data: []};
+      }
+      else if (curSection && /\S/.test(line))
+        curSection.data.push(line);
+    }
+    for (let section of sections)
+    {
+      section.key = section.header + " " + section.data[0];
+      section.data.sort();
+    }
+    sections.sort(function(a, b)
+    {
+      if (a.key < b.key)
+        return -1;
+      else if (a.key > b.key)
+        return 1;
+      else
+        return 0;
+    });
+    return sections.map(function(section) {
+      return [section.header].concat(section.data).join("\n");
+    }).join("\n");
   }
 
-  function testReadWrite(withExternal)
-  {
-    let tempFile = FileUtils.getFile("TmpD", ["temp_patterns1.ini"]);
-    let tempFile2 = FileUtils.getFile("TmpD", ["temp_patterns2.ini"]);
-    tempFile.createUnique(tempFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
-    createTempFile();
-
-    function canonize(data)
-    {
-      let curSection = null;
-      let sections = [];
-      for (let line of (data + "\n[end]").split(/[\r\n]+/))
-      {
-        if (/^\[.*\]$/.test(line))
-        {
-          if (curSection)
-            sections.push(curSection);
-
-          curSection = {header: line, data: []};
-        }
-        else if (curSection && /\S/.test(line))
-          curSection.data.push(line);
-      }
-      for (let section of sections)
-      {
-        section.key = section.header + " " + section.data[0];
-        section.data.sort();
-      }
-      sections.sort(function(a, b)
-      {
-        if (a.key < b.key)
-          return -1;
-        else if (a.key > b.key)
-          return 1;
-        else
-          return 0;
-      });
-      return sections.map(function(section) {
-        return [section.header].concat(section.data).join("\n");
-      }).join("\n");
+  return testData.then(data =>
+  {
+    tempFile.contents = data;
+    return loadFilters(tempFile);
+  }).then(() =>
+  {
+    test.equal(FilterStorage.fileProperties.version, FilterStorage.formatVersion, "File format version");
+
+    if (withExternal)
+    {
+      let subscription = new ExternalSubscription("~external~external subscription ID", "External subscription");
+      subscription.filters = [Filter.fromText("foo"), Filter.fromText("bar")];
+      FilterStorage.addSubscription(subscription);

[Wladimir Palant, 2016/10/05 20:10:22]

This was calling our public API in the original test, hence testing below that
the subscription was definitely added correctly. Now the subscription is being
added explicitly (public API isn't part of adblockpluscore) but I thought that
leaving the checks won't hurt - the point of this code is making sure that
external subscriptions aren't being saved, so we better be sure that we have an
external subscription in the data.

[kzar, 2016/10/06 05:13:38]

Acknowledged.

+
+      let externalSubscriptions = FilterStorage.subscriptions.filter(subscription => subscription instanceof ExternalSubscription);
+      test.equal(externalSubscriptions.length, 1, "Number of external subscriptions after updateExternalSubscription");
+
+      test.equal(externalSubscriptions[0].url, "~external~external subscription ID", "ID of external subscription");
+      test.equal(externalSubscriptions[0].filters.length, 2, "Number of filters in external subscription");
     }
 
-    function createTempFile()
-    {
-      let request = new XMLHttpRequest();
-      request.open("GET", "data/patterns.ini");
-      request.overrideMimeType("text/plain; charset=utf-8");
-      request.addEventListener("load", function()
-      {
-        writeToFile(tempFile, request.responseText);
-        loadFilters(tempFile, saveFile);
-      }, false);
-      request.send(null);
+    return saveFilters(tempFile2);
+  }).then(() => testData).then(expected =>
+  {
+    test.equal(canonize(tempFile2.contents), canonize(expected), "Read/write result");
+  }).catch(unexpectedError.bind(test)).then(() => test.done());
+}
+
+exports.testReadAndSaveToFile = function(test)
+{
+  testReadWrite(test, false);
+};
+
+exports.testReadAndSaveToFileWithExternalSubscription = function(test)
+{
+  testReadWrite(test, true);
+};
+
+exports.testLegacyGroups = {};
+
+for (let url of ["~wl~", "~fl~", "~eh~"])
+{
+  exports.testLegacyGroups["read empty " + url] = function(test)
+  {
+    let data = "[Subscription]\nurl=" + url;
+    let tempFile = IO.resolveFilePath("temp_patterns1.ini");
+    tempFile.contents = data;
+
+    loadFilters(tempFile, function()
+    {
+      test.equal(FilterStorage.subscriptions.length, 0, "Number of filter subscriptions");
+    }).catch(unexpectedError.bind(test)).then(() => test.done());
+  };
+
+  exports.testLegacyGroups["read non-empty " + url] = function(test)
+  {
+    let data = "[Subscription]\nurl=" + url + "\n[Subscription filters]\nfoo";
+    let tempFile = IO.resolveFilePath("temp_patterns1.ini");
+    tempFile.contents = data;
+
+    loadFilters(tempFile).then(() =>
+    {
+      test.equal(FilterStorage.subscriptions.length, 1, "Number of filter subscriptions");
+      if (FilterStorage.subscriptions.length == 1)
+      {
+        let subscription = FilterStorage.subscriptions[0];
+        test.equal(subscription.url, url, "Subscription ID");

[kzar, 2016/10/06 05:13:37]

Won't `url` always be "~eh~" by the time the test is run?

[Wladimir Palant, 2016/10/06 07:13:59]

Good question. I checked and this isn't the case - we actually run this test
with all three variants for `url`. However, changing `let url` into `var url`
will produce the behavior you expected.

[kzar, 2016/10/06 08:06:47]

Ah of course.

+        test.equal(subscription.title, null, "Subscription title");
+        test.deepEqual(subscription.defaults, null, "Default types");

[Wladimir Palant, 2016/10/05 20:10:22]

I had to modify this, the fallback for legacy groups is gone with
https://issues.adblockplus.org/ticket/3660 but this test wasn't adapted for some
reason. Default title is no more, and there are no implied defaults either.

[kzar, 2016/10/06 05:13:37]

Acknowledged.

+        test.equal(subscription.filters.length, 1, "Number of subscription filters");
+        if (subscription.filters.length == 1)
+          test.equal(subscription.filters[0].text, "foo", "First filter");
+      }
+    }).catch(unexpectedError.bind(test)).then(() => test.done());
+  };
+}
+
+exports.testReadLegacyFilters = function(test)
+{
+  let data = "[Subscription]\nurl=~user~1234\ntitle=Foo\n[Subscription filters]\n[User patterns]\nfoo\n\\[bar]\nfoo#bar";
+  let tempFile = IO.resolveFilePath("temp_patterns1.ini");
+  tempFile.contents = data;
+
+  loadFilters(tempFile).then(() =>
+  {
+    test.equal(FilterStorage.subscriptions.length, 1, "Number of filter subscriptions");
+    if (FilterStorage.subscriptions.length == 1)
+    {
+      let subscription = FilterStorage.subscriptions[0];
+      test.equal(subscription.filters.length, 3, "Number of subscription filters");
+      if (subscription.filters.length == 3)
+      {
+        test.equal(subscription.filters[0].text, "foo", "First filter");
+        test.equal(subscription.filters[1].text, "[bar]", "Second filter");
+        test.equal(subscription.filters[2].text, "foo#bar", "Third filter");
+      }
     }
-
-    function saveFile()
-    {
-      equal(FilterStorage.fileProperties.version, FilterStorage.formatVersion, "File format version");
-
-      if (withExternal)
-      {
-        let {AdblockPlus} = Cu.import(Cc["@adblockplus.org/abp/public;1"].getService(Ci.nsIURI).spec, null);
-        AdblockPlus.updateExternalSubscription("~external~external subscription ID", "External subscription", ["foo", "bar"]);
-
-        let externalSubscriptions = FilterStorage.subscriptions.filter(function (subscription) subscription instanceof ExternalSubscription);
-        equal(externalSubscriptions.length, 1, "Number of external subscriptions after updateExternalSubscription");
-
-        if (externalSubscriptions.length == 1)
-        {
-          equal(externalSubscriptions[0].url, "~external~external subscription ID", "ID of external subscription");
-          equal(externalSubscriptions[0].filters.length, 2, "Number of filters in external subscription");
-        }
-      }
-
-      saveFilters(tempFile2, compareFile);
-    }
-
-    function compareFile()
-    {
-      let stream = Cc["@mozilla.org/network/file-input-stream;1"].createInstance(Ci.nsIFileInputStream);
-      stream.init(tempFile2, FileUtils.MODE_RDONLY, FileUtils.PERMS_FILE, Ci.nsIFileInputStream.DEFER_OPEN);
-
-      NetUtil.asyncFetch(stream, function(inputStream, nsresult)
-      {
-        let result = NetUtil.readInputStreamToString(inputStream, inputStream.available(), {charset: "utf-8"});
-
-        let request = new XMLHttpRequest();
-        request.open("GET", "data/patterns.ini");
-        request.overrideMimeType("text/plain");
-        request.addEventListener("load", function()
-        {
-          let expected = request.responseText;
-          equal(canonize(result), canonize(expected), "Read/write result");
-
-          tempFile.remove(false);
-          tempFile2.remove(false);
-          start();
-        }, false);
-        request.send(null);
-      });
-    }
-  }
-
-  asyncTest("Read and save to file", testReadWrite.bind(false));
-  asyncTest("Read, add external subscription and save to file", testReadWrite.bind(true));
-
-  let groupTests = [
-    ["~wl~", "whitelist"],
-    ["~fl~", "blocking"],
-    ["~eh~", "elemhide"]
-  ];
-  for (let i = 0; i < groupTests.length; i++)
-  {
-    let [url, defaults] = groupTests[i];
-    asyncTest("Read empty legacy user-defined group (" + url + ")", function()
-    {
-      let data = "[Subscription]\nurl=" + url;
-      let tempFile = FileUtils.getFile("TmpD", ["temp_patterns1.ini"]);
-      writeToFile(tempFile, data);
-
-      loadFilters(tempFile, function()
-      {
-        tempFile.remove(false);
-        equal(FilterStorage.subscriptions.length, 0, "Number of filter subscriptions");
-        start();
-      });
-    });
-    asyncTest("Read non-empty legacy user-defined group (" + url + ")", function()
-    {
-      let data = "[Subscription]\nurl=" + url + "\n[Subscription filters]\nfoo";
-      let tempFile = FileUtils.getFile("TmpD", ["temp_patterns1.ini"]);
-      writeToFile(tempFile, data);
-
-      loadFilters(tempFile, function()
-      {
-        tempFile.remove(false);
-        equal(FilterStorage.subscriptions.length, 1, "Number of filter subscriptions");
-        if (FilterStorage.subscriptions.length == 1)
-        {
-          let subscription = FilterStorage.subscriptions[0];
-          equal(subscription.url, url, "Subscription ID");
-          equal(subscription.title, Utils.getString(defaults + "Group_title"), "Subscription title");
-          deepEqual(subscription.defaults, [defaults], "Default types");
-          equal(subscription.filters.length, 1, "Number of subscription filters");
-          if (subscription.filters.length == 1)
-            equal(subscription.filters[0].text, "foo", "First filter");
-        }
-        start();
-      });
-    });
-  }
-
-  asyncTest("Read legacy user-defined filters", function()
-  {
-    let data = "[Subscription]\nurl=~user~1234\ntitle=Foo\n[Subscription filters]\n[User patterns]\nfoo\n\\[bar]\nfoo#bar";
-    let tempFile = FileUtils.getFile("TmpD", ["temp_patterns1.ini"]);
-    writeToFile(tempFile, data);
-
-    loadFilters(tempFile, function()
-    {
-      tempFile.remove(false);
-      equal(FilterStorage.subscriptions.length, 1, "Number of filter subscriptions");
-      if (FilterStorage.subscriptions.length == 1)
-      {
-        let subscription = FilterStorage.subscriptions[0];
-        equal(subscription.filters.length, 3, "Number of subscription filters");
-        if (subscription.filters.length == 3)
-        {
-          equal(subscription.filters[0].text, "foo", "First filter");
-          equal(subscription.filters[1].text, "[bar]", "Second filter");
-          equal(subscription.filters[2].text, "foo#bar", "Third filter");
-        }
-      }
-      start();
-    });
-  });
-
-  asyncTest("Saving without backups", function()
-  {
-    Prefs.patternsbackups = 0;
-    Prefs.patternsbackupinterval = 24;
-
-    let tempFile = FileUtils.getFile("TmpD", ["temp_patterns.ini"]);
-    tempFile.createUnique(tempFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
-    FilterStorage.__defineGetter__("sourceFile", () => tempFile.clone());
-
-    saveFilters(null, function()
-    {
-      saveFilters(null, function()
-      {
-        let backupFile = tempFile.clone();
-        backupFile.leafName = backupFile.leafName.replace(/\.ini$/, "-backup1.ini");
-        ok(!backupFile.exists(), "Backup shouldn't be created");
-        start();
-      });
-    });
-  });
-
-  asyncTest("Saving with backups", function()
-  {
-    Prefs.patternsbackups = 2;
-    Prefs.patternsbackupinterval = 24;
-
-    let tempFile = FileUtils.getFile("TmpD", ["temp_patterns.ini"]);
-    tempFile.createUnique(tempFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
-    FilterStorage.__defineGetter__("sourceFile", () => tempFile.clone());
-
+  }).catch(unexpectedError.bind(test)).then(() => test.done());
+};
+
+exports.testSavingWithoutBackups = function(test)
+{
+  Prefs.patternsbackups = 0;
+  Prefs.patternsbackupinterval = 24;
+
+  let tempFile = IO.resolveFilePath("temp_patterns.ini");
+  Object.defineProperty(FilterStorage, "sourceFile", {get: () => tempFile.clone()});
+
+  saveFilters(null).then(() =>
+  {
+    return saveFilters(null);
+  }).then(() =>
+  {
     let backupFile = tempFile.clone();
     backupFile.leafName = backupFile.leafName.replace(/\.ini$/, "-backup1.ini");
-
-    let backupFile2 = tempFile.clone();
-    backupFile2.leafName = backupFile2.leafName.replace(/\.ini$/, "-backup2.ini");
-
-    let backupFile3 = tempFile.clone();
-    backupFile3.leafName = backupFile3.leafName.replace(/\.ini$/, "-backup3.ini");
-
-    let oldModifiedTime;
-
-    saveFilters(null, callback1);
-
-    function callback1()
-    {
-      // Save again immediately
-      saveFilters(null, callback2);
-    }
-
-    function callback2()
-    {
-      backupFile = backupFile.clone();  // File parameters are cached, clone to prevent this
-      ok(backupFile.exists(), "First backup created");
-
-      backupFile.lastModifiedTime -= 10000;
-      oldModifiedTime = backupFile.lastModifiedTime;
-      saveFilters(null, callback3);
-    }
-
-    function callback3()
-    {
-      backupFile = backupFile.clone();  // File parameters are cached, clone to prevent this
-      equal(backupFile.lastModifiedTime, oldModifiedTime, "Backup not overwritten if it is only 10 seconds old");
-
-      backupFile.lastModifiedTime -= 40*60*60*1000;
-      oldModifiedTime = backupFile.lastModifiedTime;
-      saveFilters(null, callback4);
-    }
-
-    function callback4()
-    {
-      backupFile = backupFile.clone();  // File parameters are cached, clone to prevent this
-      notEqual(backupFile.lastModifiedTime, oldModifiedTime, "Backup overwritten if it is 40 hours old");
-
-      backupFile2 = backupFile2.clone();  // File parameters are cached, clone to prevent this
-      ok(backupFile2.exists(), "Second backup created when first backup is overwritten");
-
-      backupFile.lastModifiedTime -= 20000;
-      oldModifiedTime = backupFile2.lastModifiedTime;
-      saveFilters(null, callback5);
-    }
-
-    function callback5()
-    {
-      backupFile2 = backupFile2.clone();  // File parameters are cached, clone to prevent this
-      equal(backupFile2.lastModifiedTime, oldModifiedTime, "Second backup not overwritten if first one is only 20 seconds old");
-
-      backupFile.lastModifiedTime -= 25*60*60*1000;
-      oldModifiedTime = backupFile2.lastModifiedTime;
-      saveFilters(null, callback6);
-    }
-
-    function callback6()
-    {
-      backupFile2 = backupFile2.clone();  // File parameters are cached, clone to prevent this
-      notEqual(backupFile2.lastModifiedTime, oldModifiedTime, "Second backup overwritten if first one is 25 hours old");
-
-      ok(!backupFile3.exists(), "Third backup not created with patternsbackups = 2");
-
-      try
-      {
-        tempFile.remove(false);
-      } catch (e) {}
-      try
-      {
-        backupFile.remove(false);
-      } catch (e) {}
-      try
-      {
-        backupFile2.remove(false);
-      } catch (e) {}
-      try
-      {
-        backupFile3.remove(false);
-      } catch (e) {}
-
-      start();
-    }
-  });
-})();
+    test.ok(!backupFile.exists(), "Backup shouldn't be created");
+  }).catch(unexpectedError.bind(test)).then(() => test.done());
+};
+
+exports.testSavingWithBackups = function(test)
+{
+  Prefs.patternsbackups = 2;
+  Prefs.patternsbackupinterval = 24;
+
+  let tempFile = IO.resolveFilePath("temp_patterns.ini");
+  Object.defineProperty(FilterStorage, "sourceFile", {get: () => tempFile.clone()});
+
+  let backupFile = tempFile.clone();
+  backupFile.leafName = backupFile.leafName.replace(/\.ini$/, "-backup1.ini");
+
+  let backupFile2 = tempFile.clone();
+  backupFile2.leafName = backupFile2.leafName.replace(/\.ini$/, "-backup2.ini");
+
+  let backupFile3 = tempFile.clone();
+  backupFile3.leafName = backupFile3.leafName.replace(/\.ini$/, "-backup3.ini");
+
+  let oldModifiedTime;
+
+  saveFilters(null).then(() =>
+  {
+    // Save again immediately
+    return saveFilters(null);
+  }).then(() =>
+  {
+    test.ok(backupFile.exists(), "First backup created");
+
+    backupFile.lastModifiedTime -= 10000;
+    oldModifiedTime = backupFile.lastModifiedTime;
+    return saveFilters(null);
+  }).then(() =>
+  {
+    test.equal(backupFile.lastModifiedTime, oldModifiedTime, "Backup not overwritten if it is only 10 seconds old");
+
+    backupFile.lastModifiedTime -= 40*60*60*1000;
+    oldModifiedTime = backupFile.lastModifiedTime;
+    return saveFilters(null);
+  }).then(() =>
+  {
+    test.notEqual(backupFile.lastModifiedTime, oldModifiedTime, "Backup overwritten if it is 40 hours old");
+
+    test.ok(backupFile2.exists(), "Second backup created when first backup is overwritten");
+
+    backupFile.lastModifiedTime -= 20000;
+    oldModifiedTime = backupFile2.lastModifiedTime;
+    return saveFilters(null);
+  }).then(() =>
+  {
+    test.equal(backupFile2.lastModifiedTime, oldModifiedTime, "Second backup not overwritten if first one is only 20 seconds old");
+
+    backupFile.lastModifiedTime -= 25*60*60*1000;
+    oldModifiedTime = backupFile2.lastModifiedTime;
+    return saveFilters(null);
+  }).then(() =>
+  {
+    test.notEqual(backupFile2.lastModifiedTime, oldModifiedTime, "Second backup overwritten if first one is 25 hours old");
+
+    test.ok(!backupFile3.exists(), "Third backup not created with patternsbackups = 2");
+  }).catch(unexpectedError.bind(test)).then(() => test.done());
+};