Skip to content

Commit

Permalink
Add typing information to selenium-webdriver in e2e task and make som…
Browse files Browse the repository at this point in the history
…e required changes (ampproject#39523)
  • Loading branch information
danielrozenberg authored Oct 6, 2023
1 parent 331ff4a commit 29acb32
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 72 deletions.
3 changes: 1 addition & 2 deletions build-system/tasks/e2e/controller-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ class ControllerPromise extends Promise {
/**
* Returns a Promise that resolves when the given expected value fulfills
* the given condition.
* @param {function(TYPE): ?TYPE} condition
* @return {!Promise<?TYPE>}
* @type {undefined|function(TYPE,function(TYPE): ?TYPE): Promise<TYPE>}
*/
this.waitForValue = opt_waitForValue;
}
Expand Down
51 changes: 26 additions & 25 deletions build-system/tasks/e2e/describes-e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ require('geckodriver');
const argv = require('minimist')(process.argv.slice(2));
const chrome = require('selenium-webdriver/chrome');
const firefox = require('selenium-webdriver/firefox');
const selenium = require('selenium-webdriver');
const {Builder, logging} = require('selenium-webdriver');
const {
clearLastExpectError,
getLastExpectError,
Expand All @@ -19,7 +19,6 @@ const {configureHelpers} = require('../../../testing/helpers');
const {HOST, PORT} = require('../serve');
const {installRepl, uninstallRepl} = require('./repl');
const {isCiBuild} = require('../../common/ci');
const {Builder, Capabilities, logging} = selenium;

/** Should have something in the name, otherwise nothing is shown. */
const SUB = ' ';
Expand All @@ -40,6 +39,9 @@ if (argv.coverage) {
istanbulMiddleware = require('istanbul-middleware/lib/core');
}

/** @typedef {import('selenium-webdriver').WebDriver} WebDriver */
/** @typedef {"chrome" | "firefox" | "safari"} BrowserNameDef */

/**
* @typedef {{
* browsers: string,
Expand Down Expand Up @@ -87,10 +89,10 @@ function getConfig() {

/**
* Configure and launch a Selenium instance
* @param {string} browserName
* @param {BrowserNameDef} browserName
* @param {!SeleniumConfigDef=} args
* @param {string=} deviceName
* @return {!selenium.WebDriver}
* @return {!WebDriver}
*/
function createSelenium(browserName, args = {}, deviceName) {
switch (browserName) {
Expand All @@ -100,24 +102,18 @@ function createSelenium(browserName, args = {}, deviceName) {
case 'firefox':
return createDriver(browserName, getFirefoxArgs(args), deviceName);
case 'chrome':
default:
return createDriver(browserName, getChromeArgs(args), deviceName);
}
}

/**
*
* @param {string} browserName
* @param {BrowserNameDef} browserName
* @param {!string[]} args
* @param {string=} deviceName
* @return {!selenium.WebDriver}
* @return {!WebDriver}
*/
function createDriver(browserName, args, deviceName) {
const capabilities = Capabilities[browserName]();

const prefs = new logging.Preferences();
prefs.setLevel(logging.Type.PERFORMANCE, logging.Level.ALL);
capabilities.setLoggingPrefs(prefs);
switch (browserName) {
case 'firefox':
const firefoxOptions = new firefox.Options();
Expand All @@ -130,22 +126,27 @@ function createDriver(browserName, args, deviceName) {
.forBrowser('firefox')
.setFirefoxOptions(firefoxOptions)
.build();

case 'chrome':
const chromeOptions = new chrome.Options(capabilities);
chromeOptions.addArguments(args);
const loggingPrefs = new logging.Preferences();
loggingPrefs.setLevel(logging.Type.PERFORMANCE, logging.Level.ALL);

const chromeOptions = new chrome.Options();
chromeOptions.setLoggingPrefs(loggingPrefs);
chromeOptions.addArguments(...args);
if (process.env.CHROME_BIN) {
chromeOptions.setChromeBinaryPath(process.env.CHROME_BIN);
}
if (deviceName) {
chromeOptions.setMobileEmulation({deviceName});
}
const driver = chrome.Driver.createSession(chromeOptions);
//TODO(estherkim): workaround. `onQuit()` was added in selenium-webdriver v4.0.0-alpha.5
//which is also when `Server terminated early with status 1` began appearing. Coincidence? Maybe.
driver.onQuit = null;
return driver;
return new Builder()
.forBrowser('chrome')
.setChromeOptions(chromeOptions)
.build();

case 'safari':
return new Builder().forBrowser(browserName).build();
return new Builder().forBrowser('safari').build();
}
}

Expand Down Expand Up @@ -430,7 +431,7 @@ function describeEnv(factory) {
}

/**
* @param {string} browserName
* @param {BrowserNameDef} browserName
*/
function createVariantDescribe(browserName) {
for (const name in variants) {
Expand All @@ -453,7 +454,7 @@ function describeEnv(factory) {
*
* @param {string} _name
* @param {object} variant
* @param {string} browserName
* @param {BrowserNameDef} browserName
*/
function doTemplate(_name, variant, browserName) {
const env = Object.create(variant);
Expand Down Expand Up @@ -567,7 +568,7 @@ class EndToEndFixture {

/**
* @param {!Object} env
* @param {string} browserName
* @param {BrowserNameDef} browserName
* @param {number} retries
* @return {Promise<void>}
*/
Expand Down Expand Up @@ -635,9 +636,9 @@ class EndToEndFixture {
/**
* Get the driver for the configured engine.
* @param {!DescribesConfigDef} describesConfig
* @param {string} browserName
* @param {BrowserNameDef} browserName
* @param {string|undefined} deviceName
* @return {!selenium.WebDriver}
* @return {!WebDriver}
*/
function getDriver({headless = false}, browserName, deviceName) {
return createSelenium(browserName, {headless}, deviceName);
Expand Down
4 changes: 2 additions & 2 deletions build-system/tasks/e2e/network-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export class NetworkLogger {

return entries.filter((entry) => {
const json = JSON.parse(entry.message);
entry.message = json.message;
return entry.message.method == networkMethod;
entry.message = json?.message;
return json.message?.method == networkMethod;
});
}

Expand Down
39 changes: 37 additions & 2 deletions build-system/tasks/e2e/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions build-system/tasks/e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"description": "amp e2e",
"devDependencies": {
"@babel/register": "7.22.15",
"@types/selenium-webdriver": "4.1.17",
"babel-regenerator-runtime": "6.5.0",
"chromedriver": "117.0.3",
"geckodriver": "4.2.1",
Expand Down
Loading

0 comments on commit 29acb32

Please sign in to comment.