Merge pull request #192 from Mr0grog/we-are-good-at-baking-cookies-bu…

…t-not-very-good-at-eating-them

Fix cookie persistence, also fix some tests that are broken in modern browsers

lib/loglevel.js

@@ -158,10 +158,12 @@
           if (typeof storedLevel === undefinedType) {
               try {
                   var cookie = window.document.cookie;
-                  var location = cookie.indexOf(
-                      encodeURIComponent(storageKey) + "=");
+                  var cookieName = encodeURIComponent(storageKey);
+                  var location = cookie.indexOf(cookieName + "=");
                   if (location !== -1) {
-                      storedLevel = /^([^;]+)/.exec(cookie.slice(location))[1];
+                      storedLevel = /^([^;]+)/.exec(
+                          cookie.slice(location + cookieName.length + 1)
+                      )[1];
                   }
               } catch (ignore) {}
           }

test/default-level-test.js

@@ -23,6 +23,10 @@ define(['test/test-helpers'], function(testHelpers) {
         });
 
         describe("If no level is saved", function() {
+            beforeEach(function () {
+                testHelpers.clearStoredLevels();
+            });
+
             it("new level is always set", function(log) {
                 log.setDefaultLevel("trace");
                 expect(log).toBeAtLevel("trace");

test/get-current-level-test.js

@@ -17,6 +17,10 @@ define(['test/test-helpers'], function(testHelpers) {
         });
 
         describe("If no level is saved", function() {
+            beforeEach(function() {
+                testHelpers.clearStoredLevels();
+            });
+
             it("current level is the default level", function(log) {
                 log.setDefaultLevel("trace");
                 expect(log.getLevel()).toBe(log.levels.TRACE);

test/local-storage-test.js

@@ -180,21 +180,23 @@ define(['test/test-helpers'], function(testHelpers) {
         describeIf(testHelpers.isCookieStorageAvailable() && testHelpers.isLocalStorageAvailable(),
                    "if localStorage and cookies are both available", function () {
 
-            it("the level stored in cookies is ignored if a local storage level is set", function () {
+            it("the level stored in cookies is ignored if a local storage level is set", function (log, done) {
                 testHelpers.setCookieStoredLevel("info");
                 testHelpers.setLocalStorageStoredLevel("debug");
 
                 testHelpers.withFreshLog(function (log) {
                     expect(log).toBeAtLevel("debug");
+                    done();
                 });
             });
 
-            it("the level stored in cookies is used if no local storage level is set", function () {
+            it("the level stored in cookies is used if no local storage level is set", function (log, done) {
                 testHelpers.setCookieStoredLevel("info");
                 window.localStorage.clear();
 
                 testHelpers.withFreshLog(function (log) {
                     expect(log).toBeAtLevel("info");
+                    done();
                 });
             });
 

test/multiple-logger-test.js

@@ -57,9 +57,24 @@ define(['test/test-helpers'], function(testHelpers) {
                 expect(function() { log.getLogger(function(){}); }).toThrow();
                 expect(function() { log.getLogger(null); }).toThrow();
                 expect(function() { log.getLogger(undefined); }).toThrow();
-                if (window.Symbol) {
-                    expect(function() { log.getLogger(Symbol()); }).toThrow();
-                }
+            });
+
+            // NOTE: this test is the same as the similarly-named test in
+            // `node-integration.js` (which only runs in Node.js). If making
+            // changes here, be sure to adjust that test as well.
+            it(typeof Symbol !== "undefined", "supports using symbols as names", function(log) {
+                var s1 = Symbol("a-symbol");
+                var s2 = Symbol("a-symbol");
+
+                var logger1 = log.getLogger(s1);
+                var defaultLevel = logger1.getLevel();
+                logger1.setLevel(log.levels.TRACE);
+
+                var logger2 = log.getLogger(s2);
+
+                // Should be unequal: same name, but different symbol instances
+                expect(logger1).not.toEqual(logger2);
+                expect(logger2.getLevel()).toEqual(defaultLevel);
             });
         });
 

test/node-integration.js

@@ -25,6 +25,9 @@ describe("loglevel included via node", function () {
         expect(console.info).toHaveBeenCalledWith("test message");
     });
 
+    // NOTE: this test is the same as the similarly-named test in
+    // `multiple-logger-test.js` (which only runs in browsers). If making
+    // changes here, be sure to adjust that test as well.
     it("supports using symbols as names", function() {
         var log = require('../lib/loglevel');
 

test/test-helpers.js

@@ -165,7 +165,7 @@ define(function () {
             window.localStorage.clear();
         }
         if (self.isCookieStorageAvailable()) {
-            var storedKeys = window.document.cookie.match(/(?:^|;\s)(loglevel(\:\w+)?)(?=\=)/g);
+            var storedKeys = window.document.cookie.match(/(?:^|;\s)(loglevel(%3a\w+)?)(?=\=)/ig);
             if (storedKeys) {
                 for (var i = 0; i < storedKeys.length; i++) {
                     window.document.cookie = storedKeys[i] + "=; expires=Thu, 01 Jan 1970 00:00:01 GMT;";
@@ -175,15 +175,15 @@ define(function () {
     };
 
     self.describeIf = function describeIf(condition, name, test) {
-        if (condition) {
-            jasmine.getEnv().describe(name, test);
-        }
+        var env = jasmine.getEnv();
+        var implementation = condition ? env.describe : env.xdescribe;
+        return implementation(name, test);
     };
 
     self.itIf = function itIf(condition, name, test) {
-        if (condition) {
-            jasmine.getEnv().it(name, test);
-        }
+        var env = jasmine.getEnv();
+        var implementation = condition ? env.it : env.xit;
+        return implementation(name, test);
     };
 
     // Forcibly reloads loglevel and asynchronously hands the resulting log to
@@ -196,9 +196,27 @@ define(function () {
         });
     };
 
-    // Wraps Jasmine's it(name, test) call to reload the loglevel dependency for the given test
-    self.itWithFreshLog = function itWithFreshLog(name, test) {
-        jasmine.getEnv().it(name, function(done) {
+    // Wraps Jasmine's `it(name, test)` call to reload the loglevel module
+    // for the given test. An optional boolean first argument causes this to
+    // behave like `itIf()` instead of `it()`.
+    //
+    // Normal usage:
+    //   itWithFreshLog("test name", function(log) {
+    //       // test code
+    //    });
+    //
+    // Conditional usage:
+    //   itWithFreshLog(shouldRunTest(), "test name", function(log) {
+    //       // test code
+    //    });
+    self.itWithFreshLog = function itWithFreshLog(condition, name, test) {
+        if (!test) {
+            test = name;
+            name = condition;
+            condition = true;
+        }
+
+        self.itIf(condition, name, function(done) {
             function runTest (log) {
                 if (test.length > 1) {
                     return test(log, done);