From bf4b0340272ffbaccab2769f9e12d703add21d56 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Wed, 23 Feb 2022 09:17:04 -0500 Subject: [PATCH] [bazel] Show CanvasKit can be loaded in POC karma_test In order to load CanvasKit, we need to add support for statically served files. On the karma side, this is done by adding an object [1] to the files list (example: [2]). Then, we need to include canvaskit.js in with the karma test files and use the callback to load canvaskit.wasm from the correct file location. [1] http://karma-runner.github.io/6.3/config/files.html [2] https://github.com/google/skia/blob/4f7b6560123d89cac2a9ec6eb71a54b5bc2fe56c/modules/canvaskit/karma.conf.js#L13 Change-Id: I7482d6e949a5e8efd0ca882efe5afbe0dc16c0e4 Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510736 Reviewed-by: Leandro Lovisolo --- bazel/karma_test.bzl | 65 ++++++++++++++++--- modules/canvaskit/BUILD.bazel | 7 ++ modules/canvaskit/Makefile | 3 + .../canvaskit/tests/bazel_canvaskitinit.js | 18 +++++ modules/canvaskit/tests/hello_world.js | 11 ++++ 5 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 modules/canvaskit/tests/bazel_canvaskitinit.js diff --git a/bazel/karma_test.bzl b/bazel/karma_test.bzl index f5edc49e13..2388ae715c 100644 --- a/bazel/karma_test.bzl +++ b/bazel/karma_test.bzl @@ -1,8 +1,10 @@ +"""This module defines rules for running JS tests in a browser.""" + # https://github.com/bazelbuild/rules_webtesting/blob/master/web/web.bzl load("@io_bazel_rules_webtesting//web:web.bzl", "web_test") load("@build_bazel_rules_nodejs//:providers.bzl", "ExternalNpmPackageInfo", "node_modules_aspect") -def karma_test(name, srcs, config_file, **kwargs): +def karma_test(name, config_file, srcs, static_files = None, **kwargs): """Tests the given JS files using Karma and a browser provided by Bazel (Chromium) This rule injects some JS code into the karma config file and produces both that modified @@ -20,12 +22,24 @@ def karma_test(name, srcs, config_file, **kwargs): situations nor bundle everything together. Args: - srcs: A list of JavaScript test files or helpers. + name: The name of the rule which actually runs the tests. generated dependent rules will use + this name plus an applicable suffix. config_file: A karma config file. The user is to expect a function called BAZEL_APPLY_SETTINGS is defined and should call it with the configuration object before passing it to config.set. + srcs: A list of JavaScript test files or helpers. + static_files: Arbitrary files which are available to be loaded. + Files are served at: + - `/static//` or + - `/static///` + Examples: + - `/static/skia/modules/canvaskit/tests/assets/color_wheel.gif` + - `/static/skia/modules/canvaskit/canvaskit_wasm/canvaskit.wasm` + **kwargs: Additional arguments are passed to @io_bazel_rules_webtesting/web_test. """ if len(srcs) == 0: fail("Must pass at least one file into srcs or there will be no tests to run") + if not static_files: + static_files = [] wrapped_test_name = name + "_karma_test" _karma_test( @@ -38,6 +52,7 @@ def karma_test(name, srcs, config_file, **kwargs): "@npm//jasmine-core", ], config_file = config_file, + static_files = static_files, visibility = ["//visibility:private"], ) @@ -59,6 +74,10 @@ def karma_test(name, srcs, config_file, **kwargs): # (e.g. the user ran `bazel run foo`). _apply_bazel_settings_js_code = """ (function(cfg) { +// This is is a JS function provided via environment variables to let us resolve files +// https://bazelbuild.github.io/rules_nodejs/Built-ins.html#nodejs_binary-templated_args +const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']); + // Apply the paths to any files that are coming from other Bazel rules (e.g. compiled JS). function addFilePaths(cfg) { if (!cfg.files) { @@ -66,6 +85,24 @@ function addFilePaths(cfg) { } cfg.files = cfg.files.concat([_BAZEL_SRCS]); cfg.basePath = "_BAZEL_BASE_PATH"; + + if (!cfg.proxies) { + cfg.proxies = {}; + } + // The following is based off of the concatjs version + // https://github.com/bazelbuild/rules_nodejs/blob/700b7a3c5f97f2877320e6e699892ee706f85269/packages/concatjs/web_test/karma.conf.js#L276 + const staticFiles = [_BAZEL_STATIC_FILES]; + for (const file of staticFiles) { + // We need to find the actual path (symlinks can apparently cause issues on Windows). + const resolvedFile = runfiles.resolve(file); + cfg.files.push({pattern: resolvedFile, included: false}); + // We want the file to be available on a path according to its location in the workspace + // (and not the path on disk), so we use a proxy to redirect. + // Prefixing the proxy path with '/absolute' allows karma to load files that are not + // underneath the basePath. This doesn't see to be an official API. + // https://github.com/karma-runner/karma/issues/2703 + cfg.proxies['/static/' + file] = '/absolute' + resolvedFile; + } } // Returns true if invoked with bazel run, i.e. the user wants to see the results on a real @@ -76,7 +113,7 @@ function isBazelRun() { } // Configures the settings to run chrome. -function applyChromiumSettings(cfg, runfiles, chromiumPath) { +function applyChromiumSettings(cfg, chromiumPath) { if (isBazelRun()) { cfg.browsers = ['Chrome']; cfg.singleRun = false; @@ -97,9 +134,6 @@ function applyChromiumSettings(cfg, runfiles, chromiumPath) { function applyBazelSettings(cfg) { addFilePaths(cfg) - // This is is a JS function provided via environment variables to let us resolve files - // https://bazelbuild.github.io/rules_nodejs/Built-ins.html#nodejs_binary-templated_args - const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']); // This is a JSON file that contains this metadata, mixed in with some other data, e.g. // the link to the correct executable for the given platform. @@ -109,7 +143,7 @@ function applyBazelSettings(cfg) { const webTestFiles = webTestMetadata['webTestFiles'][0]; const path = webTestFiles['namedFiles']['CHROMIUM']; if (path) { - applyChromiumSettings(cfg, runfiles, path); + applyChromiumSettings(cfg, path); } else { throw new Error("not supported yet"); } @@ -134,10 +168,14 @@ def _expand_templates_in_karma_config(ctx): config_segments = len(ctx.outputs.configuration.short_path.split("/")) base_path = "/".join([".."] * config_segments) + static_files = ['"{}"'.format(_absolute_path(ctx, f)) for f in ctx.files.static_files] + static_list = ", ".join(static_files) + # Replace the placeholders in the embedded JS with those files. We cannot use .format() because # the curly braces from the JS code throw it off. apply_bazel_settings = _apply_bazel_settings_js_code.replace("_BAZEL_SRCS", src_list) apply_bazel_settings = apply_bazel_settings.replace("_BAZEL_BASE_PATH", base_path) + apply_bazel_settings = apply_bazel_settings.replace("_BAZEL_STATIC_FILES", static_list) # Add in the JS fragment that applies the Bazel-specific settings to the provided config. # https://docs.bazel.build/versions/main/skylark/lib/actions.html#expand_template @@ -176,7 +214,8 @@ readonly KARMA=$(rlocation "{_KARMA_EXECUTABLE_SCRIPT}") readonly CONF=$(rlocation "{_KARMA_CONFIGURATION_FILE}") # set a temporary directory as the home directory, because otherwise Chrome fails to -# start up, complaining about a read-only file system. This does not get cleaned up automatically. +# start up, complaining about a read-only file system. This does not get cleaned up automatically +# by Bazel, so we do so after Karma finishes. export HOME=$(mktemp -d) readonly COMMAND="${{KARMA}} "start" ${{CONF}}" @@ -206,11 +245,13 @@ def _karma_test_impl(ctx): # The files that need to be included when we run the bash script that invokes Karma are: # - The templated configuration file # - Any JS test files the user provided + # - Any static files the user specified # - The other dependencies from npm (e.g. jasmine-core) runfiles = [ ctx.outputs.configuration, ] runfiles += ctx.files.srcs + runfiles += ctx.files.static_files runfiles += ctx.files.deps # We need to add the sources for our Karma dependencies as transitive dependencies, otherwise @@ -221,7 +262,7 @@ def _karma_test_impl(ctx): if ExternalNpmPackageInfo in dep: node_modules_depsets.append(dep[ExternalNpmPackageInfo].sources) else: - print("Not an external npm file?", dep) + fail("Not an external npm file: " + dep) node_modules = depset(transitive = node_modules_depsets) # https://docs.bazel.build/versions/main/skylark/lib/DefaultInfo.html @@ -250,7 +291,7 @@ _karma_test = rule( ), "deps": attr.label_list( doc = """Any karma plugins (aka peer deps) required. These are generally listed - in the provided config_file.""", + in the provided config_file""", allow_files = True, aspects = [node_modules_aspect], mandatory = True, @@ -263,6 +304,10 @@ _karma_test = rule( cfg = "exec", allow_files = True, ), + "static_files": attr.label_list( + doc = "Additional files which are available to be loaded", + allow_files = True, + ), }, outputs = { "configuration": "%{name}.conf.js", diff --git a/modules/canvaskit/BUILD.bazel b/modules/canvaskit/BUILD.bazel index a40069f7d1..b758fb8089 100644 --- a/modules/canvaskit/BUILD.bazel +++ b/modules/canvaskit/BUILD.bazel @@ -434,7 +434,14 @@ bool_flag( karma_test( name = "hello_world", srcs = [ + ":canvaskit_wasm/canvaskit.js", + # We want to make sure the CanvasKit JS is loaded before the loader script + "tests/bazel_canvaskitinit.js", + # which is loaded before the tests... "tests/hello_world.js", ], config_file = "karma.bazel.js", + static_files = [ + ":canvaskit_wasm/canvaskit.wasm", + ], ) diff --git a/modules/canvaskit/Makefile b/modules/canvaskit/Makefile index a68d568e60..05aa85fd23 100644 --- a/modules/canvaskit/Makefile +++ b/modules/canvaskit/Makefile @@ -157,3 +157,6 @@ bazel_canvaskit_release: cp ../../bazel-bin/modules/canvaskit/canvaskit_wasm/canvaskit.js build/canvaskit.js cp ../../bazel-bin/modules/canvaskit/canvaskit_wasm/canvaskit.wasm build/canvaskit.wasm ls -l build + +bazel_test_canvaskit: + bazelisk test :hello_world --compilation_mode opt --spawn_strategy=local --test_output=all diff --git a/modules/canvaskit/tests/bazel_canvaskitinit.js b/modules/canvaskit/tests/bazel_canvaskitinit.js new file mode 100644 index 0000000000..15ce82925a --- /dev/null +++ b/modules/canvaskit/tests/bazel_canvaskitinit.js @@ -0,0 +1,18 @@ +// The increased timeout is especially needed with larger binaries +// like in the debug/gpu build +jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000; + +let CanvasKit = null; +const LoadCanvasKit = new Promise((resolve, reject) => { + console.log('canvaskit loading', new Date()); + CanvasKitInit({ + locateFile: (file) => '/static/skia/modules/canvaskit/canvaskit_wasm/'+file, + }).then((loaded) => { + console.log('canvaskit loaded', new Date()); + CanvasKit = loaded; + resolve(); + }).catch((e) => { + console.error('canvaskit failed to load', new Date(), e); + reject(); + }); +}); \ No newline at end of file diff --git a/modules/canvaskit/tests/hello_world.js b/modules/canvaskit/tests/hello_world.js index 361e992a90..8f19c834d5 100644 --- a/modules/canvaskit/tests/hello_world.js +++ b/modules/canvaskit/tests/hello_world.js @@ -5,4 +5,15 @@ describe('The test harness', () => { it('runs the second test', () => { expect(null).toBeFalsy(); }); + + describe('the CanvasKit loading promise', () => { + beforeEach(async () => { + await LoadCanvasKit; + }); + + it('has access to CanvasKit', () => { + const r = CanvasKit.LTRBRect(1, 2, 3, 4); + expect(r.constructor.name).toEqual('Float32Array'); + }); + }); }) \ No newline at end of file