From 5a580307342bf59ff911a86e0bc4e01e6689b04d Mon Sep 17 00:00:00 2001 From: Matt Handley Date: Fri, 21 Aug 2020 09:36:53 -0500 Subject: [PATCH] fix(karma): allow custom browsers to specify args (fixes #595) Today, the args as specified in the various manifests of the rules_webtesting browsers never make it into the generated karma.conf.js. This results in these arguments never being used when launching Chrome, preventing customization of browsers such as window size, enabling remote debugging, or other flag-based options. This PR fixes this by reading those arguments from the manifest, and adding them to the browsers list in the generated karma.conf.js. Also, this PR includes a change to generated_file_test to allow a golden file to represent a substring of the generated output. Also Also: This PR includes a golden file test that verified that the generated karma.conf.js does read in the specified value. Furthermore, the effect of this can be verified manually via: ``` VERBOSE_LOGS=1 bazel run packages/karma/test/karma:testing_custom_chrome ``` Note the appearance of the additional flags in the new debug output. --- .bazelci/presubmit.yml | 6 ++-- internal/generated_file_test/bin.js | 32 ++++++++++++++++++- .../generated_file_test.bzl | 28 ++++++++++------ packages/karma/karma.conf.js | 14 ++++++-- packages/karma/karma_web_test.bzl | 8 ++--- packages/karma/test/karma/BUILD.bazel | 16 ++++++++++ packages/karma/test/karma/custom_chrome.json | 11 +++++++ .../karma/test/karma/karma.conf.js.golden | 5 +++ 8 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 packages/karma/test/karma/custom_chrome.json create mode 100644 packages/karma/test/karma/karma.conf.js.golden diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 08931b7c92..8cfade513f 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -32,7 +32,7 @@ tasks: # on bazelci apt-get fails with permission denied and there is no sudo # command to switch to root. # TODO(gregmagolan): figure out how to install missing shared libs - - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-cypress" + - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-browser:custom_chrome,-cypress" test_targets: - "//..." # //internal/node/test:nodejs_toolchain_linux_amd64_test is a "manual" test that must be run @@ -137,7 +137,7 @@ tasks: # on bazelci apt-get fails with permission denied and there is no sudo # command to switch to root. # TODO(gregmagolan): figure out how to install missing shared libs - - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-cypress" + - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-browser:custom_chrome,-cypress" test_targets: - "//..." # //internal/node/test:nodejs_toolchain_linux_amd64_test is a "manual" test that must be run @@ -158,7 +158,7 @@ tasks: # on bazelci apt-get fails with permission denied and there is no sudo # command to switch to root. # TODO(gregmagolan): figure out how to install missing shared libs - - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-cypress" + - "--test_tag_filters=-e2e,-examples,-manual,-browser:chromium-local,-browser:firefox-local,-browser:custom_chrome,-cypress" test_targets: - "//..." ubuntu1804_e2e: diff --git a/internal/generated_file_test/bin.js b/internal/generated_file_test/bin.js index 8385ab5e1d..0a132589dc 100644 --- a/internal/generated_file_test/bin.js +++ b/internal/generated_file_test/bin.js @@ -4,6 +4,23 @@ const path = require('path'); import * as unidiff from 'unidiff/unidiff'; const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']); +function findGoldenInGenerated(golden, actual) { + const goldenLines = golden.split(/[\r\n]+/g).map(l => l.trim()); + const actualLines = actual.split(/[\r\n]+/g).map(l => l.trim()); + // Note: this is not the fastest subsequence algorithm. + nextActualLine: for (let i = 0; i < actualLines.length; i++) { + for (let j = 0; j < goldenLines.length; j++) { + if (actualLines[i + j] !== goldenLines[j]) { + continue nextActualLine; + } + } + // A match! + return true; + } + // No match. + return false; +} + function main(args) { const [mode, golden_no_debug, golden_debug, actual] = args; const actualPath = runfiles.resolveWorkspaceRelative(actual); @@ -23,7 +40,7 @@ function main(args) { return 0; } if (mode === '--verify') { - // Generated does not match golden + // Compare the generated file to the golden file. const diff = unidiff.diffLines(goldenContents, actualContents); let prettyDiff = unidiff.formatLines(diff, {aname: `[workspace]/${golden}`, bname: `[bazel-out]/${actual}`}); @@ -41,6 +58,19 @@ If the bazel-out content is correct, you can update the workspace file by runnin `); return 1; } + if (mode === '--substring') { + // Verify that the golden file is contained _somewhere_ in the generated + // file. + const diff = findGoldenInGenerated(goldenContents, actualContents); + if (diff) { + console.error(`Unable to find golden contents inside of the the generated file: + +${goldenContents} +`) + return 1; + } + return 0; + } throw new Error('unknown mode', mode); } diff --git a/internal/generated_file_test/generated_file_test.bzl b/internal/generated_file_test/generated_file_test.bzl index 6264a8c372..ba3f8fe237 100644 --- a/internal/generated_file_test/generated_file_test.bzl +++ b/internal/generated_file_test/generated_file_test.bzl @@ -2,7 +2,7 @@ load("@build_bazel_rules_nodejs//internal/node:node.bzl", "nodejs_binary", "nodejs_test") -def generated_file_test(name, generated, src, src_dbg = None, **kwargs): +def generated_file_test(name, generated, src, substring_search = False, src_dbg = None, **kwargs): """Tests that a file generated by Bazel has identical content to a file in the workspace. This is useful for testing, where a "snapshot" or "golden" file is checked in, @@ -12,6 +12,8 @@ def generated_file_test(name, generated, src, src_dbg = None, **kwargs): name: Name of the rule. generated: a Label of the output file generated by another rule src: Label of the source file in the workspace + substring_search: When true, creates a test that will fail only if the golden file is not found + anywhere within the generated file. Note that the .update rule is not generated in substring mode. src_dbg: if the build uses `--compilation_mode dbg` then some rules will produce different output. In this case you can specify what the dbg version of the output should look like **kwargs: extra arguments passed to the underlying nodejs_test or nodejs_binary @@ -27,16 +29,22 @@ def generated_file_test(name, generated, src, src_dbg = None, **kwargs): nodejs_test( name = name, entry_point = "@build_bazel_rules_nodejs//internal/generated_file_test:bundle.js", - templated_args = ["--verify", loc % src, loc % src_dbg, loc % generated], + templated_args = [ + "--substring" if substring_search else "--verify", + loc % src, + loc % src_dbg, + loc % generated, + ], data = data, **kwargs ) - nodejs_binary( - name = name + ".update", - testonly = True, - entry_point = "@build_bazel_rules_nodejs//internal/generated_file_test:bundle.js", - templated_args = ["--out", loc % src, loc % src_dbg, loc % generated], - data = data, - **kwargs - ) + if not substring_search: + nodejs_binary( + name = name + ".update", + testonly = True, + entry_point = "@build_bazel_rules_nodejs//internal/generated_file_test:bundle.js", + templated_args = ["--out", loc % src, loc % src_dbg, loc % generated], + data = data, + **kwargs + ) diff --git a/packages/karma/karma.conf.js b/packages/karma/karma.conf.js index 0e94be0cfd..ccc78da5db 100644 --- a/packages/karma/karma.conf.js +++ b/packages/karma/karma.conf.js @@ -321,13 +321,23 @@ try { webTestNamedFiles['CHROMIUM']}' in runfiles`); } } + // Read any additional chrome options (as specified by the + // rules_webtesting manifest). + const chromeOptions = (webTestMetadata['capabilities'] || {})['goog:chromeOptions']; + const additionalArgs = (chromeOptions ? chromeOptions['args'] : []).filter(arg => { + // We never want to 'run' Chrome in headless mode. + return arg != '--headless'; + }); const browser = process.env['DISPLAY'] ? 'Chrome' : 'ChromeHeadless'; if (!supportChromeSandboxing()) { const launcher = 'CustomChromeWithoutSandbox'; - conf.customLaunchers = {[launcher]: {base: browser, flags: ['--no-sandbox']}}; + conf.customLaunchers = + {[launcher]: {base: browser, flags: ['--no-sandbox', ...additionalArgs]}}; conf.browsers.push(launcher); } else { - conf.browsers.push(browser); + const launcher = 'CustomChrome'; + conf.customLaunchers = {[launcher]: {base: browser, flags: additionalArgs}}; + conf.browsers.push(launcher); } } if (webTestNamedFiles['FIREFOX']) { diff --git a/packages/karma/karma_web_test.bzl b/packages/karma/karma_web_test.bzl index bb6220977e..58c2f1a0c4 100644 --- a/packages/karma/karma_web_test.bzl +++ b/packages/karma/karma_web_test.bzl @@ -120,10 +120,7 @@ def _find_dep(ctx, suffix): # Generates the karma configuration file for the rule def _write_karma_config(ctx, files, amd_names_shim): - configuration = ctx.actions.declare_file( - "%s.conf.js" % ctx.label.name, - sibling = ctx.outputs.executable, - ) + configuration = ctx.outputs.configuration config_file = None @@ -325,6 +322,9 @@ _karma_web_test = rule( test = True, executable = True, attrs = KARMA_WEB_TEST_ATTRS, + outputs = { + "configuration": "%{name}.conf.js", + }, ) def karma_web_test( diff --git a/packages/karma/test/karma/BUILD.bazel b/packages/karma/test/karma/BUILD.bazel index e261a3b081..aee67d3354 100644 --- a/packages/karma/test/karma/BUILD.bazel +++ b/packages/karma/test/karma/BUILD.bazel @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test") +load("@io_bazel_rules_webtesting//web:web.bzl", "custom_browser") load("//packages/karma:index.bzl", "karma_web_test_suite") karma_web_test_suite( @@ -26,6 +28,7 @@ karma_web_test_suite( browsers = [ "@io_bazel_rules_webtesting//browsers:chromium-local", "@io_bazel_rules_webtesting//browsers:firefox-local", + ":custom_chrome", ], static_files = [ "unnamed-amd-module.js", @@ -41,3 +44,16 @@ karma_web_test_suite( "requirejs-config.js", ], ) + +custom_browser( + name = "custom_chrome", + browser = "@io_bazel_rules_webtesting//browsers:chromium-local", + metadata = "custom_chrome.json", +) + +generated_file_test( + name = "test_custom_chrome_karma_conf", + src = "karma.conf.js.golden", + generated = "testing_wrapped_test.conf.js", + substring_search = True, +) diff --git a/packages/karma/test/karma/custom_chrome.json b/packages/karma/test/karma/custom_chrome.json new file mode 100644 index 0000000000..c7798d0dda --- /dev/null +++ b/packages/karma/test/karma/custom_chrome.json @@ -0,0 +1,11 @@ +{ + "capabilities": { + "goog:chromeOptions": { + "args": [ + "--remote-debugging-port=9222", + "--headless", + "--use-gl=swiftshader-webgl" + ] + } + } +} diff --git a/packages/karma/test/karma/karma.conf.js.golden b/packages/karma/test/karma/karma.conf.js.golden new file mode 100644 index 0000000000..c3bb4a92bd --- /dev/null +++ b/packages/karma/test/karma/karma.conf.js.golden @@ -0,0 +1,5 @@ +const chromeOptions = (webTestMetadata['capabilities'] || {})['goog:chromeOptions']; +const additionalArgs = (chromeOptions ? chromeOptions['args'] : []).filter(arg => { + // We never want to 'run' Chrome in headless mode. + return arg != '--headless'; +});