From 8e0f353c4bfb545e377c8e8e643660cdb987621a Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Fri, 28 Jul 2023 15:44:13 -0700 Subject: [PATCH] ci: Refactor and update Shaka lab definitions - In karma.conf.js, give ourselves the ability to merge Selenium configs together without clobbering argument lists, for better deduplication in shaka-lab.yaml. This is based on https://github.com/shaka-project/karma-local-wd-launcher/pull/50 - Refactor shaka-lab.yaml to take advantage of this new ability. This removes a bunch of duplicate command line arguments for various versions of Chrome. - Remove a TODO about a bug that Chrome decided they would never fix. - Sort the list of devices and platforms, with comments to mark out the sections. - Remove IE11, which we are well past supporting or testing. This should be cherry-picked to all active branches. --- build/shaka-lab.yaml | 134 +++++++++++++------------ docs/tutorials/selenium-grid-config.md | 68 +++++++++++-- karma.conf.js | 35 ++++++- 3 files changed, 161 insertions(+), 76 deletions(-) diff --git a/build/shaka-lab.yaml b/build/shaka-lab.yaml index 4a7256eb6d..278b2d0b33 100644 --- a/build/shaka-lab.yaml +++ b/build/shaka-lab.yaml @@ -1,7 +1,7 @@ # Selenium grid config for the Shaka lab. This is the source of truth for the # browsers and devices in the Shaka lab at Google. -# For syntax and general information, see docs/selenium-grid-config.md +# For syntax and general information, see docs/tutorials/selenium-grid-config.md # A set of variables to contain repeated configurations which can then be @@ -9,7 +9,7 @@ # generates an "anchor" with the given name. Later, you can inject the # contents of the variable with "*name". vars: - firefox_config: &firefox_config + basic_firefox_config: &basic_firefox_config moz:firefoxOptions: # Override Firefox default preferences in the temporary profile created # for each test run. @@ -20,50 +20,32 @@ vars: # Overrides Firefox's Linux-specific default setting to disable DRM. media.eme.enabled: true - minimum_chrome_args: &minimum_chrome_args - # On Chrome m59+ we can test EME on platforms with pop-up prompts, such as - # Android and ChromeOS. Note that this flag does not take a port number - # (domain vs origin). - # This flag requires setting --user-data-dir as well, however, webdriver - # already takes care of that for us (except on Android). - - "--unsafely-allow-protected-media-identifier-for-domain=karma.shakalab.rocks" - # Normally, Chrome disallows autoplaying videos in many cases. Enable it - # for testing. - - "--autoplay-policy=no-user-gesture-required" - - minimum_chrome_android_args: &minimum_chrome_android_args - # There is no way in YAML to natively merge arrays, so we start by - # duplicating the flags from minimum_chrome_args above. - - "--unsafely-allow-protected-media-identifier-for-domain=karma.shakalab.rocks" - - "--autoplay-policy=no-user-gesture-required" - # On Android we must set --user-data-dir. WebDriver does not do it for - # us as it does on other platforms. Without --user-data-dir, - # --unsafely-allow... does not work. - - "--user-data-dir=/data/data/com.android.chrome/cache" - - minimum_chromeos_args: &minimum_chromeos_args - # There is no way in YAML to natively merge arrays, so we start by - # duplicating the flags from minimum_chrome_args above. - - "--unsafely-allow-protected-media-identifier-for-domain=karma.shakalab.rocks" - - "--autoplay-policy=no-user-gesture-required" - # Allow remote attestation even though the device may be in dev mode. This - # is critical for testing involving L1 content licenses. - - "--allow-ra-in-dev-mode" - - chrome_config: &chrome_config + basic_chrome_config: &basic_chrome_config goog:chromeOptions: - args: *minimum_chrome_args + args: + # On Chrome m59+ we can test EME on platforms with pop-up prompts, such + # as Android and ChromeOS. Note that this flag does not take a port + # number (domain vs origin). This flag requires setting + # --user-data-dir as well, however, webdriver already takes care of + # that for us (except on Android). + - "--unsafely-allow-protected-media-identifier-for-domain=karma.shakalab.rocks" + # Normally, Chrome disallows autoplaying videos in many cases. Enable + # it for testing. + - "--autoplay-policy=no-user-gesture-required" # Instruct chromedriver not to disable component updater. The # component updater must run in order for the Widevine CDM to be # available when using a new user-data-dir. - # TODO(http://crbug.com/613581): Remove once Chrome bug is fixed. excludeSwitches: - "disable-component-update" - chrome_android_config: &chrome_android_config + android_chrome_config: &android_chrome_config goog:chromeOptions: - args: *minimum_chrome_android_args + args: + # On Android we must set --user-data-dir. WebDriver does not do it for + # us as it does on other platforms. Without --user-data-dir, + # --unsafely-allow... does not work. + - "--user-data-dir=/data/data/com.android.chrome/cache" # Once the new session request reaches chromedriver, it will take # the androidPackage option as a request to start Chrome through @@ -71,25 +53,49 @@ vars: androidPackage: com.android.chrome chromeos_config: &chromeos_config - # Pass these client-specified arguments through generic-webdriver-server. + # Pass Chrome arguments through generic-webdriver-server. # This array will be appended after "--" instead of being set through one # specific flag. For example, with ["--foo", "--bar=baz"] in the args, # this would generate a command like: # chromeos-webdriver-server.js -- --foo --bar=baz # Those parameters will be passed on to Chrome instead of used by the # WebDriver server. - generic:args: *minimum_chromeos_args + generic:args: + # On Chrome m59+ we can test EME on platforms with pop-up prompts, such + # as Android and ChromeOS. Note that this flag does not take a port + # number (domain vs origin). This flag requires setting + # --user-data-dir as well, however, webdriver already takes care of + # that for us (except on Android). + - "--unsafely-allow-protected-media-identifier-for-domain=karma.shakalab.rocks" + # Normally, Chrome disallows autoplaying videos in many cases. Enable + # it for testing. + - "--autoplay-policy=no-user-gesture-required" + # Allow remote attestation even though the device may be in dev mode. This + # is critical for testing involving L1 content licenses. + - "--allow-ra-in-dev-mode" + + safari_tp_config: &safari_tp_config + safari.options: + technologyPreview: true + +### Mac ### ChromeMac: browser: chrome os: Mac - extra_config: *chrome_config + extra_configs: + - *basic_chrome_config FirefoxMac: browser: firefox os: Mac - extra_config: *firefox_config + extra_configs: + - *basic_firefox_config + +EdgeMac: + browser: msedge + os: Mac Safari: browser: safari @@ -100,57 +106,56 @@ SafariTP: disabled: true browser: safari os: Mac - extra_config: - safari.options: - technologyPreview: true + extra_configs: + - *safari_tp_config -EdgeMac: - browser: msedge - os: Mac + +### Windows ### ChromeWindows: browser: chrome os: Windows - extra_config: *chrome_config + extra_configs: + - *basic_chrome_config FirefoxWindows: browser: firefox os: Windows - extra_config: *firefox_config - -IE11: - # IE11 support has been removed from the latest release branch. - disabled: true - browser: internet explorer - os: Windows - extra_config: - se:ieOptions: - # The zoom setting must be checked, or screenshot tests will be way off! - ignoreZoomSetting: false - ignoreProtectedModeSettings: true + extra_configs: + - *basic_firefox_config Edge: browser: msedge os: Windows + +### Linux ### + ChromeLinux: browser: chrome os: Linux - extra_config: *chrome_config + extra_configs: + - *basic_chrome_config FirefoxLinux: browser: firefox os: Linux - extra_config: *firefox_config + extra_configs: + - *basic_firefox_config EdgeLinux: browser: msedge os: Linux + +### Misc ### + ChromeAndroid: browser: chrome os: Android - extra_config: *chrome_android_config + extra_configs: + - *basic_chrome_config + - *android_chrome_config Chromecast: browser: chromecast @@ -160,7 +165,8 @@ Chromebook: disabled: true browser: chromeos version: Pixelbook - extra_config: *chromeos_config + extra_configs: + - *chromeos_config Tizen: browser: tizen diff --git a/docs/tutorials/selenium-grid-config.md b/docs/tutorials/selenium-grid-config.md index 6255b3c2d0..4f5a39008d 100644 --- a/docs/tutorials/selenium-grid-config.md +++ b/docs/tutorials/selenium-grid-config.md @@ -47,17 +47,20 @@ vars: FirefoxMac: browser: firefox os: Mac - extra_config: *firefox_config + extra_configs: + - *firefox_config FirefoxWindows: browser: firefox os: Windows - extra_config: *firefox_config + extra_configs: + - *firefox_config FirefoxLinux: browser: firefox os: Linux - extra_config: *firefox_config + extra_configs: + - *firefox_config ``` ### Browsers @@ -75,8 +78,8 @@ browser made available to Karma. Within each of those keys are the following: and must match the string and case used in the Selenium node config. - `disabled` (optional): If true, this browser is disabled and will not be used unless explicitly requested. - - `extra_config` (optional): A dictionary of extra configs which will be - merged with the WebDriver launcher config in Karma. + - `extra_configs` (optional): An array of dictionaries of extra configs which + will be merged with the WebDriver launcher config in Karma. Examples of basic desktop browsers definitions: @@ -96,9 +99,9 @@ Safari: SafariTP: browser: safari os: Mac - extra_config: - safari.options: - technologyPreview: true + extra_configs: + - safari.options: + technologyPreview: true ChromeWindows: browser: chrome @@ -120,3 +123,52 @@ FirefoxLinux: browser: firefox os: Linux ``` + +### Composing configs + +You can define variables for browser configs that can be composed together in +`extra_configs`. For example, below you will see some basic definitions for +Chrome arguments and parameters, and then an additional config with an argument +that is only needed for some platforms. `karma.conf.js` will merge those +argument lists intelligently when it loads the YAML config. + +```yaml +vars: + basic_chrome_config: &basic_chrome_config + goog:chromeOptions: + args: + # Normally, Chrome disallows autoplaying videos in many cases. Enable + # it for testing. + - "--autoplay-policy=no-user-gesture-required" + + # Instruct chromedriver not to disable component updater. + excludeSwitches: + - "disable-component-update" + + headless_chrome_config: &headless_chrome_config + goog:chromeOptions: + args: + # Run in headless mode. + - "--headless" + +ChromeLinux: + browser: chrome + os: Mac + extra_configs: + - *basic_chrome_config + +ChromeWindows: + browser: chrome + os: Mac + extra_configs: + - *basic_chrome_config + +ChromeMac: + browser: chrome + os: Mac + # Take the parameters and arguments for basic_chrome_config, then append the + # arguments for headless_chrome_config. + extra_configs: + - *basic_chrome_config + - *headless_chrome_config +``` diff --git a/karma.conf.js b/karma.conf.js index de08c470fa..039d1b4de6 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -7,9 +7,11 @@ // Karma configuration // Install required modules by running "npm install" -const Jimp = require('jimp'); +// lodash is an indirect dependency, depended on by Karma +const _ = require('lodash'); const fs = require('fs'); const glob = require('glob'); +const Jimp = require('jimp'); const path = require('path'); const rimraf = require('rimraf'); const {ssim} = require('ssim.js'); @@ -17,6 +19,29 @@ const util = require('karma/common/util'); const which = require('which'); const yaml = require('js-yaml'); +/** + * Like Object.assign, but recursive and doesn't clobber objects and arrays. + * If two arrays are merged, they are concatenated. + * Ex: + * mergeConfigs({ foo: 'bar', args: [1, 2, 3] }, + * { baz: 'blah', args: [4, 5, 6] }) + * => { foo: 'bar', baz: 'blah', args: [1, 2, 3, 4, 5, 6] } + * + * @param {Object} first + * @param {Object} second + * @return {Object} + */ +function mergeConfigs(first, second) { + return _.mergeWith( + first, + second, + (firstValue, secondValue) => { + if (Array.isArray(firstValue)) { + return firstValue.concat(secondValue); + } + }); +} + /** * @param {Object} config */ @@ -78,7 +103,7 @@ module.exports = (config) => { } // Add standard WebDriver configs. - Object.assign(launcher, { + mergeConfigs(launcher, { base: 'WebDriver', config: {hostname: gridHostname, port: gridPort}, pseudoActivityInterval: 20000, @@ -87,8 +112,10 @@ module.exports = (config) => { version: metadata.version, }); - if (metadata.extra_config) { - Object.assign(launcher, metadata.extra_config); + if (metadata.extra_configs) { + for (const config of metadata.extra_configs) { + mergeConfigs(launcher, config); + } } }