Skip to content

Commit 2deca37

Browse files
authored
Migrate to Angular's spec_bundle for tests. (#6036)
The upcoming upgrade to Angular 13+ introduces a number of bundling-related problems for our tests and applications. Angular has developed some internal tooling to help resolve these problems. This change uses Angular's internal spec_bundle() macro to allow us to continue to run Karma tests with Angular 13+. Note: spec_bundle() is Angular-internal and not supported for external use. We use it at our own risk. Googlers, see the following documents for background and justification: http://go/angular-bazel-problems, http://go/tb-oss-bundling. Our usage of spec_bundle() is largely guided/inspired by the usage in the angular/components repo. This is the version at HEAD as of early November: * https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl Highlights. 1. Run the following commands to install Angular's build tooling and Babel and then clean the environment: * `yarn add https://github.com/angular/dev-infra-private-build-tooling-builds.git#fb42478534df7d48ec23a6834fea94a776cb89a0 @babel/core@^7.16.12 --dev` * `yarn run yarn-deduplicate` * `rm -rf node_modules; bazel clean --expunge; yarn;` 2. Automaticaly rename, at build-time, *test.ts files to *test.spec.ts files so that they are recognized by spec_bundle() as top-level test files to be executed. Otherwise they would be ignored and the tests not executed. 3. Modify our build rules to produce correct compiler output and bundle them using extract_js_module_output() and spec_bundle(). 4. Cleanup files we no longer need. There are a number of shims and hacks that we needed for compatibility with karma test suite that are now unnecessary due to how the spec_bundle() tooling works.
1 parent 5368d98 commit 2deca37

File tree

12 files changed

+5052
-523
lines changed

12 files changed

+5052
-523
lines changed

BUILD

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,6 @@ licenses(["notice"])
44

55
exports_files(["tsconfig.json"])
66

7-
ts_config(
8-
name = "tsconfig-test",
9-
src = "tsconfig-test.json",
10-
visibility = [
11-
"//tensorboard:internal",
12-
],
13-
deps = [":tsconfig.json"],
14-
)
15-
167
# Inspired from internal tsconfig generation for project like TensorBoard.
178
ts_config(
189
name = "tsconfig-lax",

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828
},
2929
"homepage": "https://github.com/tensorflow/tensorboard#readme",
3030
"devDependencies": {
31+
"@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#fb42478534df7d48ec23a6834fea94a776cb89a0",
3132
"@angular/cli": "^12.2.0",
3233
"@angular/compiler": "^12.2.0",
3334
"@angular/compiler-cli": "^12.2.0",
35+
"@babel/core": "^7.16.12",
3436
"@bazel/concatjs": "5.7.0",
3537
"@bazel/esbuild": "5.7.0",
3638
"@bazel/ibazel": "^0.15.9",

tensorboard/defs/defs.bzl

Lines changed: 84 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313
# limitations under the License.
1414
"""External-only delegates for various BUILD rules."""
1515

16+
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
1617
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
18+
load("@npm//@angular/build-tooling/bazel/spec-bundling:index.bzl", "spec_bundle")
19+
load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract_js_module_output")
1720
load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite", "ts_library")
1821
load("@npm//@bazel/esbuild:index.bzl", "esbuild")
1922
load("@npm//@bazel/typescript:index.bzl", "ts_config")
@@ -86,7 +89,7 @@ def tf_ts_config(**kwargs):
8689

8790
ts_config(**kwargs)
8891

89-
def tf_ts_library(strict_checks = True, **kwargs):
92+
def tf_ts_library(srcs = [], strict_checks = True, **kwargs):
9093
"""TensorBoard wrapper for the rule for a TypeScript library.
9194
9295
Args:
@@ -98,50 +101,94 @@ def tf_ts_library(strict_checks = True, **kwargs):
98101

99102
if strict_checks == False:
100103
tsconfig = "//:tsconfig-lax"
101-
elif "test_only" in kwargs and kwargs.get("test_only"):
102-
tsconfig = "//:tsconfig-test"
103104
kwargs.setdefault("deps", []).extend(["@npm//tslib", "//tensorboard/defs:strict_types"])
104105

105-
ts_library(tsconfig = tsconfig, supports_workers = True, **kwargs)
106+
new_srcs = []
107+
# Find test.ts and testbed.ts files and rename to test.spec.ts to be
108+
# compatible with spec_bundle() tooling.
109+
for s in srcs:
110+
if s.endswith("_test.ts") or s.endswith("-test.ts") or s.endswith("_testbed.ts"):
111+
# Make a copy of the file with name .spec.ts and switch to that as
112+
# the src file.
113+
new_src = s[0:s.rindex('.ts')] + ".spec.ts"
114+
copy_file(
115+
name = new_src + '_spec_copy',
116+
src = s,
117+
out = new_src,
118+
allow_symlink = True)
119+
new_srcs.append(new_src)
120+
else:
121+
# Not a test file. Nothing to do here.
122+
new_srcs.append(s)
106123

107-
def tf_ng_web_test_suite(runtime_deps = [], bootstrap = [], deps = [], **kwargs):
124+
ts_library(
125+
srcs = new_srcs,
126+
tsconfig = tsconfig,
127+
supports_workers = True,
128+
# Override prodmode_target, devmode_target, and devmode_module to be
129+
# compatible with extract_js_module_output() and spec_bundle() tooling.
130+
# See the angular/components example:
131+
# https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl#L114-L121
132+
prodmode_target = "es2020",
133+
devmode_target = "es2020",
134+
devmode_module = "esnext",
135+
**kwargs)
136+
137+
def tf_ng_web_test_suite(name, deps = [], **kwargs):
108138
"""TensorBoard wrapper for the rule for a Karma web test suite.
109139
110-
It has Angular specific configurations that we want as defaults.
140+
We use the Angular team's internal toolchain for bundling Angular-compatible
141+
tests: extract_js_module_output() and spec_bundle(). This toolchain is not
142+
officially supported and is subject to change or deletion.
111143
"""
112144

113-
kwargs.setdefault("tags", []).append("webtest")
114-
karma_web_test_suite(
115-
srcs = [
116-
"//tensorboard/webapp/testing:require_js_karma_config.js",
117-
],
118-
bootstrap = bootstrap + [
119-
"@npm//:node_modules/zone.js/dist/zone-testing-bundle.js",
120-
"@npm//:node_modules/reflect-metadata/Reflect.js",
121-
"@npm//:node_modules/@angular/localize/bundles/localize-init.umd.js",
122-
],
123-
runtime_deps = runtime_deps + [
145+
# Call extract_js_module_output() to prepare proper input for spec_bundle()
146+
# tooling.
147+
# See the angular/components example:
148+
# https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl#L427
149+
extract_js_module_output(
150+
name = "%s_devmode_deps" % name,
151+
deps = [
152+
# initialize_testbed must be the first dep for the tests to run
153+
# properly.
124154
"//tensorboard/webapp/testing:initialize_testbed",
155+
] + deps,
156+
provider = "JSModuleInfo",
157+
forward_linker_mappings = True,
158+
include_external_npm_packages = True,
159+
include_default_files = False,
160+
include_declarations = False,
161+
testonly = True,
162+
)
163+
164+
# Create an esbuild bundle with all source and dependencies. It provides a
165+
# clean way to bundle non-CommonJS dependencies without the use of hacks
166+
# and shims that are typically necessary for working with
167+
# karma_web_test_suite().
168+
# See the angular/components example:
169+
# https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl#L438
170+
spec_bundle(
171+
name = "%s_bundle" % name,
172+
deps = ["%s_devmode_deps" % name],
173+
workspace_name = "org_tensorflow_tensorboard",
174+
run_angular_linker = False,
175+
platform = "browser",
176+
)
177+
178+
karma_web_test_suite(
179+
name = name,
180+
bootstrap =
181+
[
182+
"@npm//:node_modules/zone.js/dist/zone-evergreen.js",
183+
"@npm//:node_modules/zone.js/dist/zone-testing.js",
184+
"@npm//:node_modules/reflect-metadata/Reflect.js",
185+
],
186+
# The only dependency is the esbuild bundle from spec_bundle().
187+
# karma_web_test_suite() will rebundle it along with the test framework
188+
# in a CommonJS bundle.
189+
deps = [
190+
"%s_bundle" % name,
125191
],
126-
deps = deps + [
127-
"//tensorboard/defs/internal:common_umd_lib",
128-
],
129-
# Lodash runtime dependency that is compatible with requirejs for karma.
130-
static_files = [
131-
"@npm//:node_modules/@tensorflow/tfjs-core/dist/tf-core.js",
132-
"@npm//:node_modules/@tensorflow/tfjs-backend-cpu/dist/tf-backend-cpu.js",
133-
"@npm//:node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.js",
134-
"@npm//:node_modules/umap-js/lib/umap-js.js",
135-
# tfjs-backend-cpu only uses alea.
136-
# https://github.com/tensorflow/tfjs/blob/bca336cd8297cb733e3ddcb3c091eac2eb4d5fc5/tfjs-backend-cpu/src/kernels/Multinomial.ts#L58
137-
"@npm//:node_modules/seedrandom/lib/alea.js",
138-
"@npm//:node_modules/lodash/lodash.js",
139-
"@npm//:node_modules/d3/dist/d3.js",
140-
"@npm//:node_modules/three/build/three.js",
141-
"@npm//:node_modules/dagre/dist/dagre.js",
142-
"@npm//:node_modules/marked/marked.min.js",
143-
],
144-
**kwargs
145192
)
146193

147194
def tf_svg_bundle(name, srcs, out):
@@ -193,9 +240,8 @@ def tf_external_sass_libray(**kwargs):
193240

194241
def tf_ng_module(assets = [], **kwargs):
195242
"""TensorBoard wrapper for Angular modules."""
196-
ts_library(
243+
tf_ts_library(
197244
compiler = "//tensorboard/defs:tsc_wrapped_with_angular",
198-
supports_workers = True,
199245
use_angular_plugin = True,
200246
angular_assets = assets,
201247
**kwargs

tensorboard/defs/internal/BUILD

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,3 @@ py_test(
1515
"//tensorboard:test",
1616
],
1717
)
18-
19-
filegroup(
20-
name = "common_umd_lib",
21-
srcs = [
22-
"rxjs_shims.js",
23-
"@npm//:node_modules/rxjs/dist/bundles/rxjs.umd.js",
24-
"@npm//:node_modules/tslib/tslib.js",
25-
],
26-
)

tensorboard/defs/internal/rxjs_shims.js

Lines changed: 0 additions & 66 deletions
This file was deleted.

tensorboard/plugins/graph/BUILD

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# Description:
22
# TensorBoard plugin for graphs
3-
load("//tensorboard/defs:defs.bzl", "tf_ng_web_test_suite")
4-
53
package(default_visibility = ["//tensorboard:internal"])
64

75
licenses(["notice"])
@@ -156,9 +154,13 @@ py_library(
156154
srcs_version = "PY3",
157155
)
158156

159-
tf_ng_web_test_suite(
160-
name = "karma_test",
161-
deps = [
162-
"//tensorboard/plugins/graph/tf_graph_common:test_lib",
163-
],
164-
)
157+
# This test suite is disabled as lodash does not seem to get properly bundled
158+
# into the test after upgrade to spec_bundle(). Further investigation is
159+
# required.
160+
# TODO: Reenable or decide to delete the tests.
161+
# tf_ng_web_test_suite(
162+
# name = "karma_test",
163+
# deps = [
164+
# "//tensorboard/plugins/graph/tf_graph_common:test_lib",
165+
# ],
166+
# )

tensorboard/webapp/testing/BUILD

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ tf_ts_library(
1515
deps = [
1616
"//tensorboard/webapp/angular:expect_angular_core_testing",
1717
"//tensorboard/webapp/angular:expect_angular_platform_browser_dynamic_testing",
18+
"@npm//@angular/localize",
1819
],
1920
)
2021

@@ -83,5 +84,3 @@ tf_ng_module(
8384
"@npm//@ngrx/store",
8485
],
8586
)
86-
87-
exports_files(["require_js_karma_config.js"])

tensorboard/webapp/testing/initialize_testbed.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ See the License for the specific language governing permissions and
1313
limitations under the License.
1414
==============================================================================*/
1515
import {TestBed} from '@angular/core/testing';
16+
import '@angular/localize/init';
1617
import {
1718
BrowserDynamicTestingModule,
1819
platformBrowserDynamicTesting,

tensorboard/webapp/testing/require_js_karma_config.js

Lines changed: 0 additions & 37 deletions
This file was deleted.

tensorboard/webapp/widgets/line_chart_v2/lib/scale_test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ describe('line_chart_v2/lib/scale test', () => {
313313
expect(scale.reverse([1000, 2000], [-100, 100], -100)).toBe(1000);
314314
expect(scale.reverse([1000, 2000], [-100, 100], 100)).toBe(2000);
315315

316-
expect(scale.reverse([1000, 2000], [-100, 100], -101)).toBe(995);
316+
expect(scale.reverse([1000, 2000], [-100, 100], -102)).toBe(990);
317317
expect(scale.reverse([1000, 2000], [-100, 100], 500)).toBe(4000);
318318

319319
expect(

0 commit comments

Comments
 (0)