From 6a89e87496ed59343ddcac5a97c51f96c2da1e92 Mon Sep 17 00:00:00 2001
From: Nick Fitzgerald <fitzgen@gmail.com>
Date: Tue, 20 Oct 2015 10:56:03 +0530
Subject: [PATCH] Bug 1215606 - Ensure that DevToolsUtils.assert is properly
 exported; r=jsantell

---
 devtools/shared/DevToolsUtils.js            | 41 ++++++++++++---------
 devtools/shared/tests/unit/head_devtools.js | 10 ++++-
 devtools/shared/tests/unit/test_assert.js   | 36 ++++++++++++++++++
 devtools/shared/tests/unit/xpcshell.ini     |  1 +
 4 files changed, 70 insertions(+), 18 deletions(-)
 create mode 100644 devtools/shared/tests/unit/test_assert.js

diff --git a/devtools/shared/DevToolsUtils.js b/devtools/shared/DevToolsUtils.js
index a5b187d206a0..a2a03dae4aba 100644
--- a/devtools/shared/DevToolsUtils.js
+++ b/devtools/shared/DevToolsUtils.js
@@ -458,6 +458,25 @@ exports.dbg_assert = function dbg_assert(cond, e) {
   }
 };
 
+exports.defineLazyGetter(this, "AppConstants", () => {
+  const scope = {};
+  Cu.import("resource://gre/modules/AppConstants.jsm", scope);
+  return scope.AppConstants;
+});
+
+/**
+ * No operation. The empty function.
+ */
+exports.noop = function () { };
+
+function reallyAssert(condition, message) {
+  if (!condition) {
+    const err = new Error("Assertion failure: " + message);
+    exports.reportException("DevToolsUtils.assert", err);
+    throw err;
+  }
+}
+
 /**
  * DevToolsUtils.assert(condition, message)
  *
@@ -477,23 +496,11 @@ exports.dbg_assert = function dbg_assert(cond, e) {
  * This is an improvement over `dbg_assert`, which doesn't actually cause any
  * fatal behavior, and is therefore much easier to accidentally ignore.
  */
-exports.defineLazyGetter(exports, "assert", () => {
-  function noop(condition, msg) { }
-
-  function assert(condition, message) {
-    if (!condition) {
-      const err = new Error("Assertion failure: " + message);
-      exports.reportException("DevToolsUtils.assert", err);
-      throw err;
-    }
-  }
-
-  const scope = {};
-  Cu.import("resource://gre/modules/AppConstants.jsm", scope);
-  const { DEBUG, DEBUG_JS_MODULES } = scope.AppConstants;
-
-  return (DEBUG || DEBUG_JS_MODULES || exports.testing) ? assert : noop;
-});
+Object.defineProperty(exports, "assert", {
+  get: () => (AppConstants.DEBUG || AppConstants.DEBUG_JS_MODULES || this.testing)
+    ? reallyAssert
+    : exports.noop,
+})
 
 /**
  * Defines a getter on a specified object for a module.  The module will not
diff --git a/devtools/shared/tests/unit/head_devtools.js b/devtools/shared/tests/unit/head_devtools.js
index 219c1c17c3bf..6ed43690cbc3 100644
--- a/devtools/shared/tests/unit/head_devtools.js
+++ b/devtools/shared/tests/unit/head_devtools.js
@@ -9,6 +9,11 @@ const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 
 // Register a console listener, so console messages don't just disappear
 // into the ether.
+
+// If for whatever reason the test needs to post console errors that aren't
+// failures, set this to true.
+var ALLOW_CONSOLE_ERRORS = false;
+
 var errorCount = 0;
 var listener = {
   observe: function (aMessage) {
@@ -35,7 +40,10 @@ var listener = {
     while (DebuggerServer.xpcInspector.eventLoopNestLevel > 0) {
       DebuggerServer.xpcInspector.exitNestedEventLoop();
     }
-    do_throw("head_dbg.js got console message: " + string + "\n");
+
+    if (!ALLOW_CONSOLE_ERRORS) {
+      do_throw("head_devtools.js got console message: " + string + "\n");
+    }
   }
 };
 
diff --git a/devtools/shared/tests/unit/test_assert.js b/devtools/shared/tests/unit/test_assert.js
new file mode 100644
index 000000000000..e18e2c37ea49
--- /dev/null
+++ b/devtools/shared/tests/unit/test_assert.js
@@ -0,0 +1,36 @@
+/* -*- js-indent-level: 2; indent-tabs-mode: nil -*- */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test DevToolsUtils.assert
+
+ALLOW_CONSOLE_ERRORS = true;
+
+function run_test() {
+  // Enable assertions.
+  DevToolsUtils.testing = true;
+
+  const { assert } = DevToolsUtils;
+  equal(typeof assert, "function");
+
+  try {
+    assert(true, "this assertion should not fail");
+  } catch (e) {
+    // If you catch assertion failures in practice, I will hunt you down. I get
+    // email notifications every time it happens.
+    ok(false, "Should not get an error for an assertion that should not fail. Got "
+       + DevToolsUtils.safeErrorString(e));
+  }
+
+  let assertionFailed = false;
+  try {
+    assert(false, "this assertion should fail");
+  } catch (e) {
+    ok(e.message.startsWith("Assertion failure:"),
+       "Should be an assertion failure error");
+    assertionFailed = true;
+  }
+
+  ok(assertionFailed,
+     "The assertion should have failed, which should throw an error when assertions are enabled.");
+}
diff --git a/devtools/shared/tests/unit/xpcshell.ini b/devtools/shared/tests/unit/xpcshell.ini
index 81591b0e68ad..3f29cc2d638b 100644
--- a/devtools/shared/tests/unit/xpcshell.ini
+++ b/devtools/shared/tests/unit/xpcshell.ini
@@ -6,6 +6,7 @@ skip-if = toolkit == 'android' || toolkit == 'gonk'
 support-files =
   exposeLoader.js
 
+[test_assert.js]
 [test_fetch-chrome.js]
 [test_fetch-file.js]
 [test_fetch-http.js]