fix(firefox): enable more tests (puppeteer#4037)

Further align Puppeteer-Firefox implementation with Puppeteer to pass more tests.
kiku-jw · Apr 6, 2019 · 34dca2f · 34dca2f
1 parent 4cbf200
commit 34dca2f
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/experimental/puppeteer-firefox/lib/FrameManager.js b/experimental/puppeteer-firefox/lib/FrameManager.js
@@ -364,15 +364,21 @@ class Frame {
    * @param {!{timeout?: number, visible?: boolean, hidden?: boolean}=} options
    * @return {!Promise<!ElementHandle>}
    */
-  _waitForSelectorOrXPath(selectorOrXPath, isXPath, options = {}) {
+  async _waitForSelectorOrXPath(selectorOrXPath, isXPath, options = {}) {
     const {
       visible: waitForVisible = false,
       hidden: waitForHidden = false,
       timeout = this._timeoutSettings.timeout(),
     } = options;
     const polling = waitForVisible || waitForHidden ? 'raf' : 'mutation';
     const title = `${isXPath ? 'XPath' : 'selector'} "${selectorOrXPath}"${waitForHidden ? ' to be hidden' : ''}`;
-    return new WaitTask(this, predicate, title, polling, timeout, selectorOrXPath, isXPath, waitForVisible, waitForHidden).promise;
+    const waitTask = new WaitTask(this, predicate, title, polling, timeout, selectorOrXPath, isXPath, waitForVisible, waitForHidden);
+    const handle = await waitTask.promise;
+    if (!handle.asElement()) {
+      await handle.dispose();
+      return null;
+    }
+    return handle.asElement();
 
     /**
      * @param {string} selectorOrXPath
@@ -668,7 +674,7 @@ class WaitTask {
     this._frame = frame;
     this._polling = polling;
     this._timeout = timeout;
-    this._predicateBody = helper.isString(predicateBody) ? 'return ' + predicateBody : 'return (' + predicateBody + ')(...args)';
+    this._predicateBody = helper.isString(predicateBody) ? 'return (' + predicateBody + ')' : 'return (' + predicateBody + ')(...args)';
     this._args = args;
     this._runCount = 0;
     frame._waitTasks.add(this);

diff --git a/test/waittask.spec.js b/test/waittask.spec.js
@@ -60,16 +60,17 @@ module.exports.addTests = function({testRunner, expect, product, Errors}) {
       await page.waitFor(timeout);
       expect(Date.now() - startTime).not.toBeLessThan(timeout / 2);
     });
-    it_fails_ffox('should work with multiline body', async({page, server}) => {
+    it('should work with multiline body', async({page, server}) => {
       const result = await page.waitForFunction(`
         (() => true)()
       `);
       expect(await result.jsonValue()).toBe(true);
     });
     it('should wait for predicate', async({page, server}) => {
-      const watchdog = page.waitFor(() => window.innerWidth < 100);
-      page.setViewport({width: 10, height: 10});
-      await watchdog;
+      await Promise.all([
+        page.waitFor(() => window.innerWidth < 100),
+        page.setViewport({width: 10, height: 10}),
+      ]);
     });
     it('should throw when unknown type', async({page, server}) => {
       let error = null;
@@ -346,7 +347,7 @@ module.exports.addTests = function({testRunner, expect, product, Errors}) {
       expect(await waitForSelector).toBe(true);
       expect(divRemoved).toBe(true);
     });
-    it_fails_ffox('should return null if waiting to hide non-existing element', async({page, server}) => {
+    it('should return null if waiting to hide non-existing element', async({page, server}) => {
       const handle = await page.waitForSelector('non-existing', { hidden: true });
       expect(handle).toBe(null);
     });