Bug 1499096 - Update wrong usage of ok() with todo_is();r=Standard8
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 23 Oct 2018 07:13:02 +0000
changeset 490875 a82f6a836cf384325aef49402fd4473a744648aa
parent 490874 eee0effaa1f00aeb017a2c7bf107425e64222faa
child 490876 dfaaf3978e3c7f1c98fd20270a0e50e4a6ddd733
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersStandard8
bugs1499096
milestone65.0a1
Bug 1499096 - Update wrong usage of ok() with todo_is();r=Standard8 Depends on D8739. This changeset updates calls to ok() that were most likely intended for is(), but are not working as is. Differential Revision: https://phabricator.services.mozilla.com/D8740
browser/base/content/test/general/browser_visibleTabs_tabPreview.js
devtools/client/performance/test/browser_perf-recording-notices-05.js
devtools/server/tests/mochitest/test_inspector-retain.html
dom/base/test/test_sendQueryContentAndSelectionSetEvent.html
dom/url/tests/test_url.html
editor/libeditor/tests/test_bug641466.html
toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
toolkit/content/tests/chrome/test_custom_element_base.xul
toolkit/content/tests/chrome/test_tabbox.xul
--- a/browser/base/content/test/general/browser_visibleTabs_tabPreview.js
+++ b/browser/base/content/test/general/browser_visibleTabs_tabPreview.js
@@ -11,17 +11,20 @@ add_task(async function test() {
 
   // test the ctrlTab.tabList
   pressCtrlTab();
   is(ctrlTab.tabList.length, 3, "Show 3 tabs in tab preview");
   releaseCtrl();
 
   gBrowser.showOnlyTheseTabs([origTab]);
   pressCtrlTab();
-  ok(ctrlTab.tabList.length, 1, "Show 1 tab in tab preview");
+
+  // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500959
+  // `ctrlTab.tabList.length` is still equal to 3 at this step.
+  todo_is(ctrlTab.tabList.length, 1, "Show 1 tab in tab preview");
   ok(!ctrlTab.isOpen, "With 1 tab open, Ctrl+Tab doesn't open the preview panel");
 
   gBrowser.showOnlyTheseTabs([origTab, tabOne, tabTwo]);
   pressCtrlTab();
   ok(ctrlTab.isOpen, "With 3 tabs open, Ctrl+Tab does open the preview panel");
   releaseCtrl();
 
   // cleanup
--- a/devtools/client/performance/test/browser_perf-recording-notices-05.js
+++ b/devtools/client/performance/test/browser_perf-recording-notices-05.js
@@ -28,13 +28,17 @@ add_task(async function() {
 
   PerformanceController._setMultiprocessAttributes();
   is($("#performance-view").getAttribute("e10s"), "disabled",
     "When e10s is disabled, container has [e10s=disabled].");
 
   enabled = true;
 
   PerformanceController._setMultiprocessAttributes();
-  ok($("#performance-view").getAttribute("e10s"), "",
+
+  // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500913
+  // This cannot work with the current implementation, _setMultiprocessAttributes is not
+  // removing existing attributes.
+  todo_is($("#performance-view").getAttribute("e10s"), "",
     "When e10s is enabled, there should be no e10s attribute.");
 
   await teardownToolboxAndRemoveTab(panel);
 });
--- a/devtools/server/tests/mochitest/test_inspector-retain.html
+++ b/devtools/server/tests/mochitest/test_inspector-retain.html
@@ -124,28 +124,35 @@ addTest(function testWinRace() {
 });
 
 // Same as above, but issue the request right after the 'new-mutations' event, so that
 // we *lose* the race.
 addTest(function testLoseRace() {
   let front = null;
   promiseDone(gWalker.querySelector(gWalker.rootNode, "#z").then(node => {
     front = node;
-    gInspectee.querySelector("#z").parentNode = null;
-    const contentNode = gInspectee.querySelector("#a");
+    const contentNode = gInspectee.querySelector("#z");
     contentNode.remove();
     return promiseOnce(gWalker, "new-mutations");
   }).then(() => {
     // Verify that we have an outstanding request (no good way to tell that it's a
     // getMutations request, but there's nothing else it would be).
     is(gWalker._requests.length, 1, "Should have an outstanding request.");
     return gWalker.retainNode(front);
   }).then(() => ok(false, "Request should not have succeeded!"),
           (err) => {
-            ok(err, "noSuchActor", "Should have lost the race.");
+            // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in
+            // 1500960
+            // This is throwing because of
+            // `gInspectee.querySelector("#z").parentNode = null;` two blocks above...
+            // Even if you fix that, the test is still failing because "#a" was removed
+            // by the previous test. I am switching this to "#z" because I think that
+            // was the original intent. Still not failing with the expected error message
+            // Needs more work.
+            // ok(err, "noSuchActor", "Should have lost the race.");
             is(gWalker._retainedOrphans.size, 0, "Should have no more retained orphans.");
             // Don't re-throw the error.
           }).then(runNextTest));
 });
 
 addTest(function cleanup() {
   gWalker = null;
   gInspectee = null;
--- a/dom/base/test/test_sendQueryContentAndSelectionSetEvent.html
+++ b/dom/base/test/test_sendQueryContentAndSelectionSetEvent.html
@@ -132,19 +132,25 @@ function runTests()
   var width2 = {};
   var height2 = {};
   textRectArray.getCharacterRect(0, left, top, width, height);
   is(textRect.top, top.value,
      "sendQueryContentEvent(QUERY_TEXT_RECT_ARRAY) should return same top that returns QUERY_TEXT_RECT");
   is(textRect.left, left.value,
      "sendQueryContentEvent(QUERY_TEXT_RECT_ARRAY) should return same left that returns QUERY_TEXT_RECT");
   textRectArray.getCharacterRect(1, left2, top2, width2, height2);
-  ok(textRect.width, width.value + width2.value,
-     "sendQueryContentEvent(QUERY_TEXT_RECT_ARRAY) should return same width that QUERY_TEXT_RECT is returned for offset 1 and 2");
-  ok(textRect.height, height.value,
+
+  // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500961
+  // jdescottes: Bug 1467712 - wrong usage of ok(). This does not pass when switching to is():
+  // "got 16, expected 17". However on some other platforms it works as expected so we cannot
+  // use todo_is().
+  // is(textRect.width, width.value + width2.value,
+  //     "sendQueryContentEvent(QUERY_TEXT_RECT_ARRAY) should return same width that QUERY_TEXT_RECT is returned for offset 1 and 2");
+
+  is(textRect.height, height.value,
      "sendQueryContentEvent(QUERY_TEXT_RECT_ARRAY) should return same height that returns QUERY_TEXT_RECT");
 
   // QueryCharacterAtOffset
   result = gUtils.sendQueryContentEvent(gUtils.QUERY_CHARACTER_AT_POINT, 0, 0, textRectNative.left + 1, textRectNative.top + 1,
                                         gUtils.QUERY_CONTENT_FLAG_USE_NATIVE_LINE_BREAK);
   ok(result.succeeded,
      "sendQueryContentEvent(QUERY_CHARACTER_AT_POINT, QUERY_CONTENT_FLAG_USE_NATIVE_LINE_BREAK) should succeed");
   is(result.top, textRectNative.top,
--- a/dom/url/tests/test_url.html
+++ b/dom/url/tests/test_url.html
@@ -479,13 +479,15 @@
     // we don't implement a spec-compliant parser yet.
     // make sure we are bug compatible with existing implementations.
     url = new URL("data:text/html,<a href=\"http://example.org/?q\">Link</a>");
     is(url.href, "data:text/html,<a href=\"http://example.org/?q\">Link</a>");
   </script>
 
   <script>
     var u = new URL('http://www.example.org');
-    ok(u.toJSON(), 'http://www.example.org', "URL.toJSON()");
+    // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500962
+    // This would work with `http://www.example.org/` (trailing "/")
+    todo_is(u.toJSON(), 'http://www.example.org', "URL.toJSON()");
     is(JSON.stringify(u), "\"http://www.example.org/\"", "JSON.stringify(u) works");
   </script>
 </body>
 </html>
--- a/editor/libeditor/tests/test_bug641466.html
+++ b/editor/libeditor/tests/test_bug641466.html
@@ -23,17 +23,22 @@ https://bugzilla.mozilla.org/show_bug.cg
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(function() {
   function doTest(element) {
     element.focus();
     element.selectionStart = 4;
     element.selectionEnd = 4;
     synthesizeKey("KEY_Backspace", {repeat: 4});
 
-    ok(element.value, "", "4 backspaces should delete all of the characters in the " + element.localName);
+    // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500964
+    // This test is not working for several reasons:
+    // - race conditions between each event, we should wait before sending the next backspace
+    // - race conditions between the two tests
+    // - the value has an initial length of 8, not 4
+    todo_is(element.value, "", "4 backspaces should delete all of the characters in the " + element.localName);
   }
 
   doTest(document.querySelector("input"));
   doTest(document.querySelector("textarea"));
 
   SimpleTest.finish();
 });
 
--- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
+++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
@@ -962,17 +962,19 @@ decorate_task(
   async function testSaveStartupPrefsUserBranch(experiments) {
     await PreferenceExperiments.saveStartupPrefs();
 
     is(
       Services.prefs.getCharPref(`${startupPrefs}.fake.default`, "fallback value"),
       "experiment value",
       "The startup value for fake.default was set",
     );
-    ok(
+    // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500965
+    // This expects 0, but the test value is 32
+    todo_is(
       Services.prefs.getPrefType(`${startupPrefs}.fake.user`),
       Services.prefs.PREF_INVALID,
       "The startup value for fake.user was not set",
     );
 
     Services.prefs.deleteBranch(startupPrefs);
   },
 );
--- a/toolkit/content/tests/chrome/test_custom_element_base.xul
+++ b/toolkit/content/tests/chrome/test_custom_element_base.xul
@@ -99,17 +99,20 @@
       ok(false, "Non-custom element implements custom interface");
     } catch (ex) {
       ok(true, "Non-custom element implements custom interface");
     }
 
     // Try various ways to get the custom interface.
 
     let asControl = twoElement.getCustomInterfaceCallback(Ci.nsIDOMXULControlElement);
-    ok(asControl, twoElement, "getCustomInterface returns interface implementation ");
+
+    // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500967
+    // Not sure if this was suppose to simply check for existence or equality?
+    todo_is(asControl, twoElement, "getCustomInterface returns interface implementation ");
 
     asControl = twoElement.QueryInterface(Ci.nsIDOMXULControlElement);
     ok(asControl, "QueryInterface to nsIDOMXULControlElement");
     ok(asControl instanceof Node, "Control is a Node");
 
     // Now make sure that the custom element handles focus/tabIndex as needed by shitfing
     // focus around and enabling/disabling the simple elements.
 
--- a/toolkit/content/tests/chrome/test_tabbox.xul
+++ b/toolkit/content/tests/chrome/test_tabbox.xul
@@ -190,17 +190,21 @@ function test_tabbox_focus()
     $("tab-nofocus-button").focus();
 
     when_tab_focused(tab, function () {
       is(document.activeElement, tab, "focus in tab with no focusable elements, but with something in another tab focused");
 
       var textboxExtra = $("textbox-extra");
       textboxExtra.addEventListener("focus", function () {
         textboxExtra.removeEventListener("focus", arguments.callee, true);
-        ok(document.activeElement, textboxExtra, "focus in tab with focus currently in textbox that is sibling of tabs");
+
+        // XXX: Switched to from ok() to todo_is() in Bug 1467712. Follow up in 1500971
+        // Here the active element is not the XUL textbox but an HTML input inside it.
+        // textboxExtra is document.activeElement.parentNode.parentNode.
+        todo_is(document.activeElement, textboxExtra, "focus in tab with focus currently in textbox that is sibling of tabs");
 
         SimpleTest.finish();
       }, true);
 
       tabbox.selectedIndex = 0;
       textboxExtra.hidden = false;
       synthesizeMouseAtCenter(tab, { });
     });