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.
bazel-contrib · Sep 23, 2020 · 5a58030 · 5a58030
1 parent 2923766
commit 5a58030
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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';
+});