Skip to content

Commit

Permalink
chore(firefox): mark all Puppeteer tests that are failing on FF (#3924)
Browse files Browse the repository at this point in the history
This patch:
- introduces new testRunner methods `addTestDSL` and `addSuiteDSL`
  to add annotated test / suite.
- introduces new test/suite declaration methods: `it_fails_ffox` and
  `describe_fails_ffox`. These are equal to `it`/`describe` for chromium
  tests and to `xit`/`xdescribe` for firefox.
- marks all unsupported tests with `it_fails_ffox`
- adds a new command-line flag `'--firefox-status'` to `//test/test.js`.
  This flag dumps current amount of tests that are intentionally skipped
  for Firefox.

End goal: get rid of all `it_fails_ffox` and `describe_fails_ffox`
tests.

Drive-By: remove cookie tests  "afterEach" hook that was removing
cookies - it's not needed any more since every test is run in a
designated browser context.

References #3889
  • Loading branch information
aslushnikov committed Feb 6, 2019
1 parent 86783c2 commit 14fb3e3
Show file tree
Hide file tree
Showing 26 changed files with 230 additions and 202 deletions.
4 changes: 2 additions & 2 deletions test/accessibility.spec.js
Expand Up @@ -15,11 +15,11 @@
*/

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner;
const {it, fit, xit} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Accessibility', function() {
describe_fails_ffox('Accessibility', function() {
it('should work', async function({page}) {
await page.setContent(`
<head>
Expand Down
8 changes: 4 additions & 4 deletions test/browser.spec.js
Expand Up @@ -14,13 +14,13 @@
* limitations under the License.
*/

module.exports.addTests = function({testRunner, expect, headless, puppeteer, FFOX}) {
module.exports.addTests = function({testRunner, expect, headless, puppeteer}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Browser.target', function() {
it('should return browser target', async({browser}) => {
it_fails_ffox('should return browser target', async({browser}) => {
const target = browser.target();
expect(target.type()).toBe('browser');
});
Expand All @@ -31,7 +31,7 @@ module.exports.addTests = function({testRunner, expect, headless, puppeteer, FFO
const process = await browser.process();
expect(process.pid).toBeGreaterThan(0);
});
(FFOX ? xit : it)('should not return child_process for remote browser', async function({browser}) {
it_fails_ffox('should not return child_process for remote browser', async function({browser}) {
const browserWSEndpoint = browser.wsEndpoint();
const remoteBrowser = await puppeteer.connect({browserWSEndpoint});
expect(remoteBrowser.process()).toBe(null);
Expand Down
4 changes: 2 additions & 2 deletions test/browsercontext.spec.js
Expand Up @@ -18,7 +18,7 @@ const utils = require('./utils');

module.exports.addTests = function({testRunner, expect, puppeteer, Errors}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
const {TimeoutError} = Errors;

Expand Down Expand Up @@ -141,7 +141,7 @@ module.exports.addTests = function({testRunner, expect, puppeteer, Errors}) {
]);
expect(browser.browserContexts().length).toBe(1);
});
it('should work across sessions', async function({browser, server}) {
it_fails_ffox('should work across sessions', async function({browser, server}) {
expect(browser.browserContexts().length).toBe(1);
const context = await browser.createIncognitoBrowserContext();
expect(browser.browserContexts().length).toBe(2);
Expand Down
6 changes: 3 additions & 3 deletions test/click.spec.js
Expand Up @@ -20,15 +20,15 @@ const iPhone = DeviceDescriptors['iPhone 6'];

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
describe('Page.click', function() {
it('should click the button', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.click('button');
expect(await page.evaluate(() => result)).toBe('Clicked');
});
it('should click the button if window.Node is removed', async({page, server}) => {
it_fails_ffox('should click the button if window.Node is removed', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.evaluate(() => delete window.Node);
await page.click('button');
Expand All @@ -41,7 +41,7 @@ module.exports.addTests = function({testRunner, expect}) {
await page.click('button');
expect(await page.evaluate(() => result)).toBe('Clicked');
});
it('should click with disabled javascript', async({page, server}) => {
it_fails_ffox('should click with disabled javascript', async({page, server}) => {
await page.setJavaScriptEnabled(false);
await page.goto(server.PREFIX + '/wrappedlink.html');
await Promise.all([
Expand Down
9 changes: 2 additions & 7 deletions test/cookies.spec.js
Expand Up @@ -15,16 +15,11 @@
*/

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner;
const {it, fit, xit} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Cookies', function() {
afterEach(async({page, server}) => {
const cookies = await page.cookies(server.PREFIX + '/grid.html', server.CROSS_PROCESS_PREFIX);
for (const cookie of cookies)
await page.deleteCookie(cookie);
});
describe_fails_ffox('Cookies', function() {
it('should set and get cookies', async({page, server}) => {
await page.goto(server.PREFIX + '/grid.html');
expect(await page.cookies()).toEqual([]);
Expand Down
8 changes: 4 additions & 4 deletions test/elementhandle.spec.js
Expand Up @@ -17,8 +17,8 @@
const utils = require('./utils');

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('ElementHandle.boundingBox', function() {
Expand All @@ -29,7 +29,7 @@ module.exports.addTests = function({testRunner, expect}) {
const box = await elementHandle.boundingBox();
expect(box).toEqual({ x: 100, y: 50, width: 50, height: 50 });
});
it('should handle nested frames', async({page, server}) => {
it_fails_ffox('should handle nested frames', async({page, server}) => {
await page.setViewport({width: 500, height: 500});
await page.goto(server.PREFIX + '/frames/nested-frames.html');
const nestedFrame = page.frames()[1].childFrames()[1];
Expand Down Expand Up @@ -66,7 +66,7 @@ module.exports.addTests = function({testRunner, expect}) {
});
});

describe('ElementHandle.boxModel', function() {
describe_fails_ffox('ElementHandle.boxModel', function() {
it('should work', async({page, server}) => {
await page.goto(server.PREFIX + '/resetcss.html');

Expand Down
10 changes: 5 additions & 5 deletions test/emulation.spec.js
Expand Up @@ -20,8 +20,8 @@ const iPhone = DeviceDescriptors['iPhone 6'];
const iPhoneLandscape = DeviceDescriptors['iPhone 6 landscape'];

module.exports.addTests = function({testRunner, expect, product}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Page.viewport', function() {
Expand Down Expand Up @@ -72,7 +72,7 @@ module.exports.addTests = function({testRunner, expect, product}) {
await page.addScriptTag({url: server.PREFIX + '/modernizr.js'});
expect(await page.evaluate(() => Modernizr.touchevents)).toBe(true);
});
it('should support landscape emulation', async({page, server}) => {
it_fails_ffox('should support landscape emulation', async({page, server}) => {
await page.goto(server.PREFIX + '/mobile.html');
expect(await page.evaluate(() => screen.orientation.type)).toBe('portrait-primary');
await page.setViewport(iPhoneLandscape.viewport);
Expand All @@ -82,7 +82,7 @@ module.exports.addTests = function({testRunner, expect, product}) {
});
});

describe('Page.emulate', function() {
describe_fails_ffox('Page.emulate', function() {
it('should work', async({page, server}) => {
await page.goto(server.PREFIX + '/mobile.html');
await page.emulate(iPhone);
Expand All @@ -99,7 +99,7 @@ module.exports.addTests = function({testRunner, expect, product}) {
});
});

describe('Page.emulateMedia', function() {
describe_fails_ffox('Page.emulateMedia', function() {
it('should work', async({page, server}) => {
expect(await page.evaluate(() => window.matchMedia('screen').matches)).toBe(true);
expect(await page.evaluate(() => window.matchMedia('print').matches)).toBe(false);
Expand Down
20 changes: 10 additions & 10 deletions test/evaluation.spec.js
Expand Up @@ -23,9 +23,9 @@ try {
asyncawait = false;
}

module.exports.addTests = function({testRunner, expect, FFOX}) {
module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Page.evaluate', function() {
Expand Down Expand Up @@ -65,10 +65,10 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
await page.goto(server.PREFIX + '/global-var.html');
expect(await page.evaluate('globalVar')).toBe(123);
});
(FFOX ? xit : it)('should return undefined for objects with symbols', async({page, server}) => {
it_fails_ffox('should return undefined for objects with symbols', async({page, server}) => {
expect(await page.evaluate(() => [Symbol('foo4')])).toBe(undefined);
});
(asyncawait ? it : xit)('should work with function shorthands', async({page, server}) => {
(asyncawait ? it_fails_ffox : xit)('should work with function shorthands', async({page, server}) => {
// trick node6 transpiler to not touch our object.
// TODO(lushnikov): remove eval once Node6 is dropped.
const a = eval(`({
Expand Down Expand Up @@ -101,7 +101,7 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
await page.goto(server.EMPTY_PAGE);
expect(await frameEvaluation).toBe(42);
});
it('should work from-inside an exposed function', async({page, server}) => {
it_fails_ffox('should work from-inside an exposed function', async({page, server}) => {
// Setup inpage callback, which calls Page.evaluate
await page.exposeFunction('callController', async function(a, b) {
return await page.evaluate((a, b) => a * b, a, b);
Expand Down Expand Up @@ -158,7 +158,7 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
it('should properly serialize null fields', async({page}) => {
expect(await page.evaluate(() => ({a: undefined}))).toEqual({});
});
it('should return undefined for non-serializable objects', async({page, server}) => {
it_fails_ffox('should return undefined for non-serializable objects', async({page, server}) => {
expect(await page.evaluate(() => window)).toBe(undefined);
expect(await page.evaluate(() => [Symbol('foo4')])).toBe(undefined);
});
Expand Down Expand Up @@ -189,7 +189,7 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
const text = await page.evaluate(e => e.textContent, element);
expect(text).toBe('42');
});
it('should throw if underlying element was disposed', async({page, server}) => {
it_fails_ffox('should throw if underlying element was disposed', async({page, server}) => {
await page.setContent('<section>39</section>');
const element = await page.$('section');
expect(element).toBeTruthy();
Expand All @@ -198,15 +198,15 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
await page.evaluate(e => e.textContent, element).catch(e => error = e);
expect(error.message).toContain('JSHandle is disposed');
});
it('should throw if elementHandles are from other frames', async({page, server}) => {
it_fails_ffox('should throw if elementHandles are from other frames', async({page, server}) => {
await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
const bodyHandle = await page.frames()[1].$('body');
let error = null;
await page.evaluate(body => body.innerHTML, bodyHandle).catch(e => error = e);
expect(error).toBeTruthy();
expect(error.message).toContain('JSHandles can be evaluated only in the context they were created');
});
it('should simulate a user gesture', async({page, server}) => {
it_fails_ffox('should simulate a user gesture', async({page, server}) => {
await page.evaluate(playAudio);
// also test evaluating strings
await page.evaluate(`(${playAudio})()`);
Expand All @@ -218,7 +218,7 @@ module.exports.addTests = function({testRunner, expect, FFOX}) {
return audio.play();
}
});
it('should throw a nice error after a navigation', async({page, server}) => {
it_fails_ffox('should throw a nice error after a navigation', async({page, server}) => {
const executionContext = await page.mainFrame().executionContext();

await Promise.all([
Expand Down
8 changes: 4 additions & 4 deletions test/frame.spec.js
Expand Up @@ -18,11 +18,11 @@ const utils = require('./utils');

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Frame.executionContext', function() {
it('should work', async({page, server}) => {
it_fails_ffox('should work', async({page, server}) => {
await page.goto(server.EMPTY_PAGE);
await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
expect(page.frames().length).toBe(2);
Expand Down Expand Up @@ -58,7 +58,7 @@ module.exports.addTests = function({testRunner, expect}) {
});

describe('Frame.evaluate', function() {
it('should throw for detached frames', async({page, server}) => {
it_fails_ffox('should throw for detached frames', async({page, server}) => {
const frame1 = await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
await utils.detachFrame(page, 'frame1');
let error = null;
Expand All @@ -68,7 +68,7 @@ module.exports.addTests = function({testRunner, expect}) {
});

describe('Frame Management', function() {
it('should handle nested frames', async({page, server}) => {
it_fails_ffox('should handle nested frames', async({page, server}) => {
await page.goto(server.PREFIX + '/frames/nested-frames.html');
expect(utils.dumpFrames(page.mainFrame())).toBeGolden('nested-frames.txt');
});
Expand Down
8 changes: 4 additions & 4 deletions test/ignorehttpserrors.spec.js
Expand Up @@ -16,7 +16,7 @@

module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, puppeteer}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
describe('ignoreHTTPSErrors', function() {
beforeAll(async state => {
Expand All @@ -34,15 +34,15 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
await state.page.close();
delete state.page;
});
it('should work', async({page, httpsServer}) => {
it_fails_ffox('should work', async({page, httpsServer}) => {
let error = null;
const response = await page.goto(httpsServer.EMPTY_PAGE).catch(e => error = e);
expect(error).toBe(null);
expect(response.ok()).toBe(true);
expect(response.securityDetails()).toBeTruthy();
expect(response.securityDetails().protocol()).toBe('TLS 1.2');
});
it('Network redirects should report SecurityDetails', async({page, httpsServer}) => {
it_fails_ffox('Network redirects should report SecurityDetails', async({page, httpsServer}) => {
httpsServer.setRedirect('/plzredirect', '/empty.html');
const responses = [];
page.on('response', response => responses.push(response));
Expand All @@ -52,7 +52,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
const securityDetails = responses[0].securityDetails();
expect(securityDetails.protocol()).toBe('TLS 1.2');
});
it('should work with request interception', async({page, server, httpsServer}) => {
it_fails_ffox('should work with request interception', async({page, server, httpsServer}) => {
await page.setRequestInterception(true);
page.on('request', request => request.continue());
const response = await page.goto(httpsServer.EMPTY_PAGE);
Expand Down
6 changes: 3 additions & 3 deletions test/input.spec.js
Expand Up @@ -18,10 +18,10 @@ const path = require('path');

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
describe('input', function() {
it('should upload the file', async({page, server}) => {
it_fails_ffox('should upload the file', async({page, server}) => {
await page.goto(server.PREFIX + '/input/fileupload.html');
const filePath = path.relative(process.cwd(), __dirname + '/assets/file-to-upload.txt');
const input = await page.$('input');
Expand All @@ -34,7 +34,7 @@ module.exports.addTests = function({testRunner, expect}) {
return promise.then(() => reader.result);
}, input)).toBe('contents of the file');
});
it('keyboard.modifiers()', async({page, server}) => {
it_fails_ffox('keyboard.modifiers()', async({page, server}) => {
const keyboard = page.keyboard;
expect(keyboard._modifiers).toBe(0);
await keyboard.down('Shift');
Expand Down
10 changes: 5 additions & 5 deletions test/jshandle.spec.js
Expand Up @@ -16,11 +16,11 @@

module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner;
const {it, fit, xit, it_fails_ffox} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('Page.evaluateHandle', function() {
it('should work', async({page, server}) => {
it_fails_ffox('should work', async({page, server}) => {
const windowHandle = await page.evaluateHandle(() => window);
expect(windowHandle).toBeTruthy();
});
Expand All @@ -34,7 +34,7 @@ module.exports.addTests = function({testRunner, expect}) {
const isFive = await page.evaluate(e => Object.is(e, 5), aHandle);
expect(isFive).toBeTruthy();
});
it('should warn on nested object handles', async({page, server}) => {
it_fails_ffox('should warn on nested object handles', async({page, server}) => {
const aHandle = await page.evaluateHandle(() => document.body);
let error = null;
await page.evaluateHandle(
Expand Down Expand Up @@ -81,12 +81,12 @@ module.exports.addTests = function({testRunner, expect}) {
const json = await aHandle.jsonValue();
expect(json).toEqual({foo: 'bar'});
});
it('should not work with dates', async({page, server}) => {
it_fails_ffox('should not work with dates', async({page, server}) => {
const dateHandle = await page.evaluateHandle(() => new Date('2017-09-26T00:00:00.000Z'));
const json = await dateHandle.jsonValue();
expect(json).toEqual({});
});
it('should throw for circular objects', async({page, server}) => {
it_fails_ffox('should throw for circular objects', async({page, server}) => {
const windowHandle = await page.evaluateHandle('window');
let error = null;
await windowHandle.jsonValue().catch(e => error = e);
Expand Down

0 comments on commit 14fb3e3

Please sign in to comment.