Warn when given unsupported product name. (#5845)
* Warn when given unsupported product name. Fixes #5844. This change means when a user launches Puppeteer with a product name that is not supported (which at the time of this commit means it's not `firefox` or `chrome) we will warn them about it. Decided on just a warning vs an error because the current behaviour is that we fallback to launching Chrome and I don't think this warrants a breaking change.
This commit is contained in:
parent
6099272612
commit
b38bb4334f
@ -82,6 +82,7 @@
|
|||||||
"pixelmatch": "^4.0.2",
|
"pixelmatch": "^4.0.2",
|
||||||
"pngjs": "^5.0.0",
|
"pngjs": "^5.0.0",
|
||||||
"prettier": "^2.0.5",
|
"prettier": "^2.0.5",
|
||||||
|
"sinon": "^9.0.2",
|
||||||
"text-diff": "^1.0.1",
|
"text-diff": "^1.0.1",
|
||||||
"typescript": "3.8.3"
|
"typescript": "3.8.3"
|
||||||
},
|
},
|
||||||
|
@ -1027,6 +1027,15 @@ function Launcher(
|
|||||||
);
|
);
|
||||||
case 'chrome':
|
case 'chrome':
|
||||||
default:
|
default:
|
||||||
|
if (typeof product !== 'undefined' && product !== 'chrome') {
|
||||||
|
/* The user gave us an incorrect product name
|
||||||
|
* we'll default to launching Chrome, but log to the console
|
||||||
|
* to let the user know (they've probably typoed).
|
||||||
|
*/
|
||||||
|
console.warn(
|
||||||
|
`Warning: unknown product name ${product}. Falling back to chrome.`
|
||||||
|
);
|
||||||
|
}
|
||||||
return new ChromeLauncher(
|
return new ChromeLauncher(
|
||||||
projectRoot,
|
projectRoot,
|
||||||
preferredRevision,
|
preferredRevision,
|
||||||
|
@ -16,6 +16,7 @@
|
|||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const os = require('os');
|
const os = require('os');
|
||||||
const path = require('path');
|
const path = require('path');
|
||||||
|
const sinon = require('sinon');
|
||||||
const { helper } = require('../lib/helper');
|
const { helper } = require('../lib/helper');
|
||||||
const rmAsync = helper.promisify(require('rimraf'));
|
const rmAsync = helper.promisify(require('rimraf'));
|
||||||
const mkdtempAsync = helper.promisify(fs.mkdtemp);
|
const mkdtempAsync = helper.promisify(fs.mkdtemp);
|
||||||
@ -444,6 +445,19 @@ describe('Launcher specs', function () {
|
|||||||
expect(userAgent).toContain('Chrome');
|
expect(userAgent).toContain('Chrome');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('falls back to launching chrome if there is an unknown product but logs a warning', async () => {
|
||||||
|
const { puppeteer } = getTestState();
|
||||||
|
const consoleStub = sinon.stub(console, 'warn');
|
||||||
|
const browser = await puppeteer.launch({ product: 'SO_NOT_A_PRODUCT' });
|
||||||
|
const userAgent = await browser.userAgent();
|
||||||
|
await browser.close();
|
||||||
|
expect(userAgent).toContain('Chrome');
|
||||||
|
expect(consoleStub.callCount).toEqual(1);
|
||||||
|
expect(consoleStub.firstCall.args).toEqual([
|
||||||
|
'Warning: unknown product name SO_NOT_A_PRODUCT. Falling back to chrome.',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
/* We think there's a bug in the FF Windows launcher, or some
|
/* We think there's a bug in the FF Windows launcher, or some
|
||||||
* combo of that plus it running on CI, but we're deferring fixing
|
* combo of that plus it running on CI, but we're deferring fixing
|
||||||
* this so we can get Windows CI stable and then dig into this
|
* this so we can get Windows CI stable and then dig into this
|
||||||
|
@ -18,6 +18,7 @@ const { TestServer } = require('../utils/testserver/index');
|
|||||||
const path = require('path');
|
const path = require('path');
|
||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const os = require('os');
|
const os = require('os');
|
||||||
|
const sinon = require('sinon');
|
||||||
const puppeteer = require('../');
|
const puppeteer = require('../');
|
||||||
const utils = require('./utils');
|
const utils = require('./utils');
|
||||||
const { trackCoverage } = require('./coverage-utils');
|
const { trackCoverage } = require('./coverage-utils');
|
||||||
@ -169,3 +170,7 @@ after(async () => {
|
|||||||
await state.httpsServer.stop();
|
await state.httpsServer.stop();
|
||||||
state.httpsServer = null;
|
state.httpsServer = null;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
sinon.restore();
|
||||||
|
});
|
||||||
|
Loading…
Reference in New Issue
Block a user