Skip to content

Commit 069e097

Browse files
lauraharkercopybara-github
lauraharker
authored and
copybara-github
committed
Improve coverage of runtime library injection in JSC unit tests
Previously: during a regular full compilation, the transpilation passes would assert runtime libraries were correctly injected, but during unit tests, they would not check. Changing CompilerTestCase to run with "RECORD_AND_VALIDATE_FIELDS" mode fixes that. Requires updating transpilation tests to actually run the library injection, & tweaking the InjectTranspliationRuntimeLibraries constructor to be more testable. PiperOrigin-RevId: 892493868
1 parent 1eba23b commit 069e097

13 files changed

+104
-41
lines changed

‎src/com/google/javascript/jscomp/Es6RewriteRestAndSpread.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,6 @@ private void visitRestParam(NodeTraversal t, Node restParam, Node paramList) {
114114
functionBody.addChildToBack(let);
115115
}
116116
NodeUtil.addFeatureToScript(t.getCurrentScript(), Feature.LET_DECLARATIONS, compiler);
117-
// TODO: b/421971366 - we should instead assert that es6/util/restargument was already injected
118-
// before this point, by InjectTranspilationRuntimeLibraries.
119-
compiler
120-
.getRuntimeJsLibManager()
121-
.ensureLibraryInjected("es6/util/restarguments", /* force= */ false);
122117
t.reportCodeChange();
123118
}
124119

‎src/com/google/javascript/jscomp/InjectTranspilationRuntimeLibraries.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.javascript.jscomp;
1717

18+
import com.google.common.annotations.VisibleForTesting;
1819
import com.google.javascript.jscomp.js.RuntimeJsLibManager;
1920
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
2021
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
@@ -36,11 +37,19 @@ public final class InjectTranspilationRuntimeLibraries implements CompilerPass {
3637

3738
private final RuntimeJsLibManager runtimeLibs;
3839
private boolean injectedClassExtendsLibraries;
40+
private final boolean shouldInstrumentAsyncContext;
3941

4042
public InjectTranspilationRuntimeLibraries(AbstractCompiler compiler) {
43+
this(compiler, compiler.getOptions().getInstrumentAsyncContext());
44+
}
45+
46+
@VisibleForTesting
47+
InjectTranspilationRuntimeLibraries(
48+
AbstractCompiler compiler, boolean shouldInstrumentAsyncContext) {
4149
this.compiler = compiler;
4250
this.runtimeLibs = compiler.getRuntimeJsLibManager();
4351
this.injectedClassExtendsLibraries = false;
52+
this.shouldInstrumentAsyncContext = shouldInstrumentAsyncContext;
4453
}
4554

4655
@Override
@@ -138,7 +147,7 @@ public void process(Node externs, Node root) {
138147
runtimeLibs.injectLibForField("$jscomp.getRestArguments");
139148
}
140149

141-
if (compiler.getOptions().getInstrumentAsyncContext()
150+
if (shouldInstrumentAsyncContext
142151
// NOTE: async functions only matter for output features, since we don't bother
143152
// instrumenting them if they're being transpiled away. Generators are relevant
144153
// regardless of whether they're transpiled or not.

‎test/com/google/javascript/jscomp/CompilerTestCase.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,8 @@ protected CompilerOptions getOptions() {
385385
if (debugLoggingEnabled) {
386386
CompilerTestCaseUtils.setDebugLogDirectoryOn(options);
387387
}
388-
options.setRuntimeLibraryMode(RuntimeJsLibManager.RuntimeLibraryMode.RECORD_ONLY);
388+
options.setRuntimeLibraryMode(
389+
RuntimeJsLibManager.RuntimeLibraryMode.RECORD_AND_VALIDATE_FIELDS);
389390

390391
return options;
391392
}

‎test/com/google/javascript/jscomp/Es6ForOfConverterTest.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ public void customSetUp() throws Exception {
5151

5252
@Override
5353
protected CompilerPass getProcessor(final Compiler compiler) {
54-
return new Es6ForOfConverter(compiler);
54+
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
55+
optimizer.addOneTimePass(
56+
makePassFactory(
57+
"injectTranspilationRuntimeLibraries", InjectTranspilationRuntimeLibraries::new));
58+
optimizer.addOneTimePass(makePassFactory("es6ForOfConverter", Es6ForOfConverter::new));
59+
return optimizer;
5560
}
5661

5762
@Test

‎test/com/google/javascript/jscomp/Es6RewriteClassTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public void customSetUp() throws Exception {
7070
@Override
7171
protected CompilerPass getProcessor(final Compiler compiler) {
7272
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
73+
optimizer.addOneTimePass(
74+
makePassFactory(
75+
"injectTranspilationRuntimeLibraries", InjectTranspilationRuntimeLibraries::new));
7376
optimizer.addOneTimePass(makePassFactory("es6NormalizeClasses", Es6NormalizeClasses::new));
7477
optimizer.addOneTimePass(makePassFactory("es6ConvertSuper", Es6ConvertSuper::new));
7578
optimizer.addOneTimePass(
@@ -2807,7 +2810,6 @@ public void testClassGenerator() {
28072810
let C = function() {};
28082811
C.prototype.foo = function*() { yield 1;};
28092812
""");
2810-
assertThat(getLastCompiler().getRuntimeJsLibManager().getInjectedLibraries()).isEmpty();
28112813
}
28122814

28132815
@Test

‎test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,18 @@ protected CompilerOptions getOptions() {
7676

7777
@Override
7878
protected CompilerPass getProcessor(Compiler compiler) {
79-
return new Es6RewriteDestructuring.Builder(compiler)
80-
.setDestructuringRewriteMode(destructuringRewriteMode)
81-
.build();
79+
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
80+
optimizer.addOneTimePass(
81+
makePassFactory(
82+
"injectTranspilationRuntimeLibraries", InjectTranspilationRuntimeLibraries::new));
83+
optimizer.addOneTimePass(
84+
makePassFactory(
85+
"es6RewriteDestructuring",
86+
(c) ->
87+
new Es6RewriteDestructuring.Builder(c)
88+
.setDestructuringRewriteMode(destructuringRewriteMode)
89+
.build()));
90+
return optimizer;
8291
}
8392

8493
@Test

‎test/com/google/javascript/jscomp/Es6RewriteGeneratorsTest.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
2121

22+
import com.google.common.collect.ImmutableList;
2223
import com.google.common.collect.ImmutableMap;
2324
import com.google.common.collect.Iterables;
2425
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
@@ -60,6 +61,8 @@ public final class Es6RewriteGeneratorsTest extends CompilerTestCase {
6061
"GEN_FUNC", "$jscomp$generator$function$",
6162
"GEN_THIS", "$jscomp$generator$this$");
6263

64+
private ImmutableList<String> extraRuntimeLibFields = ImmutableList.of();
65+
6366
public Es6RewriteGeneratorsTest() {
6467
super(
6568
new TestExternsBuilder()
@@ -96,6 +99,7 @@ public void setUp() throws Exception {
9699
replaceTypesWithColors();
97100
enableMultistageCompilation();
98101
setGenericNameReplacements(SPECIAL_VARIABLES_MAP);
102+
extraRuntimeLibFields = ImmutableList.of();
99103
}
100104

101105
@Override
@@ -107,7 +111,22 @@ protected CompilerOptions getOptions() {
107111

108112
@Override
109113
protected CompilerPass getProcessor(final Compiler compiler) {
110-
return new Es6RewriteGenerators(compiler);
114+
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
115+
optimizer.addOneTimePass(
116+
makePassFactory(
117+
"extraRuntimeLibraries",
118+
(AbstractCompiler c) -> {
119+
return (externs, js) -> {
120+
for (String field : extraRuntimeLibFields) {
121+
c.getRuntimeJsLibManager().injectLibForField(field);
122+
}
123+
};
124+
}));
125+
optimizer.addOneTimePass(
126+
makePassFactory(
127+
"injectTranspilationRuntimeLibraries", InjectTranspilationRuntimeLibraries::new));
128+
optimizer.addOneTimePass(makePassFactory("es6RewriteGenerators", Es6RewriteGenerators::new));
129+
return optimizer;
111130
}
112131

113132
private void rewriteGeneratorBody(String beforeBody, String afterBody) {
@@ -174,6 +193,9 @@ private void rewriteGeneratorSwitchBodyWithVars(
174193

175194
@Test
176195
public void testGeneratorForAsyncFunction() {
196+
// Needed for the hardcoded `$jscomp.asyncExecutePromiseGeneratorProgram` reference, since we
197+
// are not actually transpiling any async functions.
198+
extraRuntimeLibFields = ImmutableList.of("$jscomp.asyncExecutePromiseGeneratorProgram");
177199
test(
178200
"""
179201
f = function() {
@@ -354,6 +376,9 @@ function f(o) {
354376

355377
@Test
356378
public void testContainsClosureAndTranspiledFromAsync() {
379+
// Needed for the hardcoded `$jscomp.asyncExecutePromiseGeneratorProgram` reference, since we
380+
// are not actually transpiling any async functions.
381+
extraRuntimeLibFields = ImmutableList.of("$jscomp.asyncExecutePromiseGeneratorProgram");
357382
test(
358383
"""
359384
function f(o) {

‎test/com/google/javascript/jscomp/Es6RewriteRestAndSpreadTest.java‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package com.google.javascript.jscomp;
1717

18-
import static com.google.common.truth.Truth.assertThat;
1918
import static org.junit.Assert.assertThrows;
2019

2120
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
@@ -40,7 +39,10 @@ public Es6RewriteRestAndSpreadTest() {
4039

4140
@Override
4241
protected CompilerPass getProcessor(Compiler compiler) {
43-
return new Es6RewriteRestAndSpread(compiler);
42+
return (externs, root) -> {
43+
new InjectTranspilationRuntimeLibraries(compiler).process(externs, root);
44+
new Es6RewriteRestAndSpread(compiler).process(externs, root);
45+
};
4446
}
4547

4648
@Before
@@ -569,8 +571,6 @@ function f() {
569571
return zero;
570572
}
571573
""");
572-
assertThat(getLastCompiler().getRuntimeJsLibManager().getInjectedLibraries())
573-
.containsExactly("es6/util/restarguments");
574574
}
575575

576576
@Test
@@ -643,8 +643,6 @@ function f(zero, one) {
643643
return two;
644644
}
645645
""");
646-
assertThat(getLastCompiler().getRuntimeJsLibManager().getInjectedLibraries())
647-
.containsExactly("es6/util/restarguments");
648646
}
649647

650648
@Test
@@ -657,8 +655,6 @@ public void testUsedRestParameterAtPositionTwoWithTypingOnFunctionVariable() {
657655
return two;
658656
}
659657
""");
660-
assertThat(getLastCompiler().getRuntimeJsLibManager().getInjectedLibraries())
661-
.containsExactly("es6/util/restarguments");
662658
}
663659

664660
@Test
@@ -684,20 +680,4 @@ function f(zero, one) {
684680
}
685681
""");
686682
}
687-
688-
@Test
689-
public void testRuntimeLibraryInjectionOccurs() {
690-
test(
691-
"function f(...rest) { return rest; }",
692-
"""
693-
function f() {
694-
let rest = $jscomp.getRestArguments.apply(0, arguments);
695-
return rest;
696-
}
697-
""");
698-
// TODO: b/421971366 - we should instead assert that es6/util/restarguments was already injected
699-
// before this point, by InjectTranspilationRuntimeLibraries.
700-
assertThat(getLastCompiler().getRuntimeJsLibManager().getInjectedLibraries())
701-
.containsExactly("es6/util/restarguments");
702-
}
703683
}

‎test/com/google/javascript/jscomp/InstrumentAsyncContextTest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,18 @@ public void setUp() throws Exception {
101101

102102
@Override
103103
protected CompilerPass getProcessor(Compiler compiler) {
104-
return new InstrumentAsyncContext(compiler, instrumentAwait);
104+
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
105+
optimizer.addOneTimePass(
106+
makePassFactory(
107+
"injectTranspilationRuntimeLibraries",
108+
(c) ->
109+
new InjectTranspilationRuntimeLibraries(
110+
compiler, /* shouldInstrumentAsyncContext= */ true)));
111+
optimizer.addOneTimePass(
112+
makePassFactory(
113+
"instrumentAsyncContext",
114+
(c) -> new InstrumentAsyncContext(compiler, instrumentAwait)));
115+
return optimizer;
105116
}
106117

107118
@Override

‎test/com/google/javascript/jscomp/LateEs6ToEs3ConverterTest.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ public void customSetUp() throws Exception {
6161

6262
@Override
6363
protected CompilerPass getProcessor(final Compiler compiler) {
64-
return new LateEs6ToEs3Converter(compiler);
64+
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
65+
optimizer.addOneTimePass(
66+
makePassFactory(
67+
"injectTranspilationRuntimeLibraries", InjectTranspilationRuntimeLibraries::new));
68+
optimizer.addOneTimePass(makePassFactory("lateEs6ToEs3Converter", LateEs6ToEs3Converter::new));
69+
return optimizer;
6570
}
6671

6772
@Test

0 commit comments

Comments
 (0)
⚡