Bug 1467712 - Fail if SimpleTest ok() is called with more than 2 arguments;r=Standard8

Depends on D10417 Differential Revision: https://phabricator.services.mozilla.com/D10418 --HG-- extra : moz-landing-system : lando
2018-11-01 13:50:27 +00:00 · 2018-11-01 13:50:27 +00:00 · 260b1e16a3
--- a/browser/base/content/test/forms/browser_selectpopup.js
+++ b/browser/base/content/test/forms/browser_selectpopup.js
@ -499,7 +499,7 @@ async function performLargePopupTests(win) {

  // Dragging above the popup scrolls it up.
  EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" }, win);
-  ok(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at drag up from option");
+  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at drag up from option");

  scrollPos = selectPopup.scrollBox.scrollTop;
  EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" }, win);
--- a/browser/base/content/test/general/browser_bug521216.js
+++ b/browser/base/content/test/general/browser_bug521216.js
@ -11,7 +11,7 @@ function test() {
  BrowserTestUtils.addTab(gBrowser, "data:text/html,<html><head><link href='about:logo' rel='shortcut icon'>");
 }

-function record(aName) {
+function recordEvent(aName) {
  info("got " + aName);
  if (!actual.includes(aName))
    actual.push(aName);
@ -31,20 +31,20 @@ function record(aName) {

 function TabOpen(aEvent) {
  if (aEvent.target == tab)
-    record("TabOpen");
+    recordEvent("TabOpen");
 }

 var progressListener = {
  onLocationChange: function onLocationChange(aBrowser) {
    if (aBrowser == tab.linkedBrowser)
-      record("onLocationChange");
+      recordEvent("onLocationChange");
  },
  onStateChange: function onStateChange(aBrowser) {
    if (aBrowser == tab.linkedBrowser)
-      record("onStateChange");
+      recordEvent("onStateChange");
  },
  onLinkIconAvailable: function onLinkIconAvailable(aBrowser) {
    if (aBrowser == tab.linkedBrowser)
-      record("onLinkIconAvailable");
+      recordEvent("onLinkIconAvailable");
  },
 };
--- a/dom/browser-element/mochitest/browserElement_LoadEvents.js
+++ b/dom/browser-element/mochitest/browserElement_LoadEvents.js
@ -38,7 +38,11 @@ function runTest() {
    seenLocationChange = true;
    ok(seenLoadStart, 'Location change after load start.');
    ok(!seenLoadEnd, 'Location change before load end.');
-    ok(e.detail.url, browserElementTestHelpers.emptyPage1, "event's reported location");
+    // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1503862
+    // Fails with: event's reported location -
+    //   got "http://example.com/tests/dom/browser-element/mochitest/file_browserElement_LoadEvents.html",
+    //   expected "http://example.com/tests/dom/browser-element/mochitest/file_empty.html"
+    todo_is(e.detail.url, browserElementTestHelpers.emptyPage1, "event's reported location");
  }

  function loadend(e) {
@ -91,7 +95,7 @@ function runTest2() {
    seenLocationChange = true;
    ok(seenLoadStart, 'Location change after load start.');
    ok(!seenLoadEnd, 'Location change before load end.');
-    ok(e.detail.url, browserElementTestHelpers.emptyPage2, "event's reported location");
+    is(e.detail.url, browserElementTestHelpers.emptyPage2, "event's reported location");
  });

  iframe.addEventListener('mozbrowserloadend', function(e) {
--- a/dom/tests/mochitest/ajax/mochikit/tests/SimpleTest/SimpleTest.js
+++ b/dom/tests/mochitest/ajax/mochikit/tests/SimpleTest/SimpleTest.js
@ -33,7 +33,16 @@ SimpleTest._stopOnLoad = true;
 /**
 * Something like assert.
 **/
-SimpleTest.ok = function (condition, name, diag) {
+SimpleTest.ok = function (condition, name) {
+    if (arguments.length > 2) {
+        const diag = "Too many arguments passed to `ok(condition, name)`";
+        SimpleTest.record(false, name, diag);
+    } else {
+        SimpleTest.record(condition, name);
+    }
+}
+
+SimpleTest.record = function (condition, name, diag) {
    var test = {'result': !!condition, 'name': name, 'diag': diag || ""};
    if (SimpleTest._logEnabled)
        SimpleTest._logResult(test, "TEST-PASS", "TEST-UNEXPECTED-FAIL");
@ -45,12 +54,12 @@ SimpleTest.ok = function (condition, name, diag) {
 **/
 SimpleTest.is = function (a, b, name) {
    var repr = MochiKit.Base.repr;
-    SimpleTest.ok(a == b, name, "got " + repr(a) + ", expected " + repr(b));
+    SimpleTest.record(a == b, name, "got " + repr(a) + ", expected " + repr(b));
 };

 SimpleTest.isnot = function (a, b, name) {
    var repr = MochiKit.Base.repr;
-    SimpleTest.ok(a != b, name, "Didn't expect " + repr(a) + ", but got it.");
+    SimpleTest.record(a != b, name, "Didn't expect " + repr(a) + ", but got it.");
 };

 //  --------------- Test.Builder/Test.More todo() -----------------
@ -424,11 +433,11 @@ SimpleTest.isa = function (object, clas) {
 };

 if ( parent.SimpleTest && parent.runAJAXTest ) (function(){
-    var oldOK = SimpleTest.ok;
+    var oldRecord = SimpleTest.record;

-    SimpleTest.ok = function(condition, name, diag) {
-        parent.SimpleTest.ok( condition, name, diag );
-        return oldOK( condition, name, diag );
+    SimpleTest.record = function(condition, name, diag, stack) {
+        parent.SimpleTest.record( condition, name, diag, stack );
+        return oldRecord( condition, name, diag, stack );
    };

    var oldFinish = SimpleTest.finish;
@ -441,6 +450,7 @@ if ( parent.SimpleTest && parent.runAJAXTest ) (function(){

 // Global symbols:
 var ok = SimpleTest.ok;
+var record = SimpleTest.record;
 var is = SimpleTest.is;
 var isnot = SimpleTest.isnot;
 var todo = SimpleTest.todo;
--- a/testing/mochitest/BrowserTestUtils/ContentTask.jsm
+++ b/testing/mochitest/BrowserTestUtils/ContentTask.jsm
@ -113,7 +113,7 @@ var ContentMessageListener = {
      }
    } else if (aMessage.name == "content-task:test-result") {
      let data = aMessage.data;
-      ContentTask._testScope.ok(data.condition, data.name, null, data.stack);
+      ContentTask._testScope.record(data.condition, data.name, null, data.stack);
    } else if (aMessage.name == "content-task:test-info") {
      ContentTask._testScope.info(aMessage.data.name);
    } else if (aMessage.name == "content-task:test-todo") {
--- a/testing/mochitest/browser-test.js
+++ b/testing/mochitest/browser-test.js
@ -17,7 +17,7 @@ ChromeUtils.defineModuleGetter(this, "ContentSearch",
  "resource:///modules/ContentSearch.jsm");

 const SIMPLETEST_OVERRIDES =
-  ["ok", "is", "isnot", "todo", "todo_is", "todo_isnot", "info", "expectAssertions", "requestCompleteLog"];
+  ["ok", "record", "is", "isnot", "todo", "todo_is", "todo_isnot", "info", "expectAssertions", "requestCompleteLog"];

 setTimeout(testInit, 0);

@ -1289,7 +1289,15 @@ function testScope(aTester, aTest, expected) {
  aTest.allowFailure = expected == "fail";

  var self = this;
-  this.ok = function test_ok(condition, name, ex, stack) {
+  this.ok = function test_ok(condition, name) {
+    if (arguments.length > 2) {
+      const ex = "Too many arguments passed to ok(condition, name)`.";
+      self.record(false, name, ex);
+    } else {
+      self.record(condition, name);
+    }
+  };
+  this.record = function test_record(condition, name, ex, stack) {
    aTest.addResult(new testResult({
      name, pass: condition, ex,
      stack: stack || Components.stack.caller,
@ -1297,11 +1305,11 @@ function testScope(aTester, aTest, expected) {
    }));
  };
  this.is = function test_is(a, b, name) {
-    self.ok(a == b, name, "Got " + a + ", expected " + b, false,
+    self.record(a == b, name, "Got " + a + ", expected " + b, false,
            Components.stack.caller);
  };
  this.isnot = function test_isnot(a, b, name) {
-    self.ok(a != b, name, "Didn't expect " + a + ", but got it", false,
+    self.record(a != b, name, "Didn't expect " + a + ", but got it", false,
            Components.stack.caller);
  };
  this.todo = function test_todo(condition, name, ex, stack) {
--- a/testing/mochitest/tests/Harness_sanity/test_sanitySimpletest.html
+++ b/testing/mochitest/tests/Harness_sanity/test_sanitySimpletest.html
@ -23,20 +23,21 @@ function starttest() {
    function() {
      //test log
      info("Logging some info")
-    
+
      //basic usage
-      ok(true, "test ok", "This should be true");
+      ok(true, "test ok");
+      SimpleTest.record(true, "test ok", "diagnostic information");
      is(0, 0, "is() test failed");
      isnot(0, 1, "isnot() test failed");
-    
+
      //todo tests
      todo(false, "test todo", "todo() test should not pass");
      todo_is(false, true, "test todo_is");
      todo_isnot(true, true, "test todo_isnot");
-    
+
      //misc
      SimpleTest.requestLongerTimeout(1);
-    
+
      //note: this test may alter runtimes as it waits
      var check = false;
      $('textB').focus();
@ -55,7 +56,7 @@ function starttest() {
          manipulateElements();
        }
      );
-    
+
      //use helper functions
      function manipulateElements()
      {
--- a/testing/mochitest/tests/SimpleTest/SimpleTest.js
+++ b/testing/mochitest/tests/SimpleTest/SimpleTest.js
@ -267,8 +267,16 @@ SimpleTest.setExpected();
 /**
 * Something like assert.
 **/
-SimpleTest.ok = function (condition, name, diag, stack = null) {
+SimpleTest.ok = function (condition, name) {
+    if (arguments.length > 2) {
+      const diag = "Too many arguments passed to `ok(condition, name)`";
+      SimpleTest.record(false, name, diag);
+    } else {
+      SimpleTest.record(condition, name);
+    }
+};

+SimpleTest.record = function (condition, name, diag, stack) {
    var test = {'result': !!condition, 'name': name, 'diag': diag};
    if (SimpleTest.expected == 'fail') {
      if (!test.result) {
@ -309,19 +317,19 @@ SimpleTest.is = function (a, b, name) {
    // Be lazy and use Object.is til we want to test a browser without it.
    var pass = Object.is(a, b);
    var diag = pass ? "" : "got " + repr(a) + ", expected " + repr(b)
-    SimpleTest.ok(pass, name, diag);
+    SimpleTest.record(pass, name, diag);
 };

 SimpleTest.isfuzzy = function (a, b, epsilon, name) {
  var pass = (a >= b - epsilon) && (a <= b + epsilon);
  var diag = pass ? "" : "got " + repr(a) + ", expected " + repr(b) + " epsilon: +/- " + repr(epsilon)
-  SimpleTest.ok(pass, name, diag);
+  SimpleTest.record(pass, name, diag);
 };

 SimpleTest.isnot = function (a, b, name) {
    var pass = !Object.is(a, b);
    var diag = pass ? "" : "didn't expect " + repr(a) + ", but got it";
-    SimpleTest.ok(pass, name, diag);
+    SimpleTest.record(pass, name, diag);
 };

 /**
@ -1577,9 +1585,9 @@ SimpleTest.isDeeply = function (it, as, name) {
    var stack = [{ vals: [it, as] }];
    var seen = [];
    if ( SimpleTest._deepCheck(it, as, stack, seen)) {
-        SimpleTest.ok(true, name);
+        SimpleTest.record(true, name);
    } else {
-        SimpleTest.ok(false, name, SimpleTest._formatStack(stack));
+        SimpleTest.record(false, name, SimpleTest._formatStack(stack));
    }
 };

@ -1602,6 +1610,7 @@ SimpleTest.isa = function (object, clas) {

 // Global symbols:
 var ok = SimpleTest.ok;
+var record = SimpleTest.record;
 var is = SimpleTest.is;
 var isfuzzy = SimpleTest.isfuzzy;
 var isnot = SimpleTest.isnot;
@ -1631,8 +1640,9 @@ window.onerror = function simpletestOnerror(errorMsg, url, lineNumber,
    }
    if (!SimpleTest._ignoringAllUncaughtExceptions) {
        // Don't log if SimpleTest.finish() is already called, it would cause failures
-        if (!SimpleTest._alreadyFinished)
-          SimpleTest.ok(isExpected, message, error);
+        if (!SimpleTest._alreadyFinished) {
+            SimpleTest.record(isExpected, message, error);
+        }
        SimpleTest._expectingUncaughtException = false;
    } else {
        SimpleTest.todo(false, message + ": " + error);
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/browser-test.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/browser-test.js
@ -43,6 +43,7 @@ module.exports = {
    "isnot": false,
    "ok": false,
    "privateNoteIntentionalCrash": false,
+    "record": false,
    "registerCleanupFunction": false,
    "requestLongerTimeout": false,
    "setExpectedFailuresForSelfTest": false,