Skip to content

Commit

Permalink
Make ExportTestFunctions work in goog.modules
Browse files Browse the repository at this point in the history
This requires:
 - rewriting test functions matching a regex in modules as well as scripts
 - looking for requires of "goog.testing.testSuite" instead of just matching the qualified name

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=246195346
  • Loading branch information
lauraharker authored and brad4d committed May 2, 2019
1 parent 215eea2 commit 352f9c8
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 11 deletions.
54 changes: 43 additions & 11 deletions src/com/google/javascript/jscomp/ExportTestFunctions.java
Expand Up @@ -33,6 +33,7 @@ public class ExportTestFunctions implements CompilerPass {
Pattern.compile(
"^(?:((\\w+\\.)+prototype\\.||window\\.)*"
+ "(setUpPage|setUp|shouldRunTests|tearDown|tearDownPage|test[\\w\\$]+))$");
private static final String GOOG_TESTING_TEST_SUITE = "goog.testing.testSuite";

private final AbstractCompiler compiler;
private final String exportSymbolFunction;
Expand Down Expand Up @@ -61,7 +62,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
return;
}

if (parent.isScript()) {
if (parent.isScript() || parent.isModuleBody()) {
if (NodeUtil.isFunctionDeclaration(n)) {
// Check for a test function statement.
String functionName = NodeUtil.getName(n);
Expand All @@ -82,10 +83,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
} else if (n.isClass()) {
exportClass(parent, n);
}
} else if (NodeUtil.isExprAssign(parent) && !n.getLastChild().isAssign()) {
} else if (NodeUtil.isExprAssign(parent)) {
// Check for a test method assignment.
Node grandparent = parent.getParent();
if (grandparent != null && grandparent.isScript()) {
if (grandparent != null && (grandparent.isScript() || grandparent.isModuleBody())) {
// NAME/(GETPROP -> ... -> NAME)
// SCRIPT -> EXPR_RESULT -> ASSIGN ->
// FUNCTION/CLASS
Expand All @@ -105,8 +106,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
exportClass(grandparent, lastChild, nodeName, parent);
}
}
} else if (n.isObjectLit()
&& isCallTargetQName(n.getParent(), "goog.testing.testSuite")) {
} else if (isTestSuiteArgument(n, t)) {
for (Node c : n.children()) {
if (c.isStringKey() && !c.isQuotedString()) {
c.setQuotedString();
Expand Down Expand Up @@ -166,12 +166,6 @@ private void rewriteMemberDefInObjLit(Node memberDef, Node objLit) {
compiler.reportChangeToEnclosingScope(objLit);
}

// TODO(johnlenz): move test suite declaration into the
// coding convention class.
private boolean isCallTargetQName(Node n, String qname) {
return (n.isCall() && n.getFirstChild().matchesQualifiedName(qname));
}

/**
* Get the node that corresponds to an expression declared with var, let or const.
* This has the AST structure VAR/LET/CONST -> NAME -> NODE
Expand Down Expand Up @@ -266,6 +260,44 @@ private void exportTestFunctionAsProperty(String fullyQualifiedFunctionName,
compiler.reportChangeToEnclosingScope(exportCall);
}

/**
* Whether this is an object literal containing test methods passed to goog.testing.testSuite
*
* @param t We only need the scope from this NodeTraversal but want to lazily create the scope.
*/
private static boolean isTestSuiteArgument(Node n, NodeTraversal t) {
return n.isObjectLit()
&& n.getParent().isCall()
&& n.isSecondChildOf(n.getParent())
&& isGoogTestingTestSuite(t, n.getPrevious());
}

/**
* Whether this node is a reference to goog.testing.testSuite, either through a goog.require or by
* its fully qualified name.
*
* <p>If this becomes more broadly useful consider branching it into its own class and making more
* robust to handle forwardDeclares, destructuring requires, etc.
*/
private static boolean isGoogTestingTestSuite(NodeTraversal t, Node qname) {
if (!qname.isQualifiedName()) {
return false;
}
Node root = NodeUtil.getRootOfQualifiedName(qname);
String rootName = root.getString();
Var rootVar = t.getScope().getSlot(rootName);
if (rootVar == null || rootVar.isGlobal()) {
return qname.matchesQualifiedName(GOOG_TESTING_TEST_SUITE);
} else if (rootVar.getScope().isModuleScope()) {
Node originalValue = rootVar.getInitialValue();
return originalValue != null
&& originalValue.isCall()
&& originalValue.getFirstChild().matchesQualifiedName("goog.require")
&& originalValue.hasTwoChildren()
&& originalValue.getSecondChild().getString().equals(GOOG_TESTING_TEST_SUITE);
}
return false;
}

/**
* Whether a function is recognized as a test function. We follow the JsUnit
Expand Down
90 changes: 90 additions & 0 deletions test/com/google/javascript/jscomp/ExportTestFunctionsTest.java
Expand Up @@ -97,6 +97,14 @@ public void testBasicTestFunctionsAreExported() {
testSame("var testCase = {}; testCase.setUpPage = function() {}");
}

@Test
public void testBasicTestFunctionsAreExported_inGoogModule() {
testSame("goog.module('test'); function Foo() {function testA(){}}");
test(
"goog.module('test'); function setUp() {}",
"goog.module('test'); function setUp(){} google_exportSymbol('setUp',setUp)");
}

/**
* Make sure this works for global functions declared as function expressions:
*
Expand Down Expand Up @@ -133,6 +141,14 @@ public void testFunctionExpressionsAreExported() {
+ "google_exportSymbol('testBar',testBar)");
}

@Test
public void testFunctionExpressionsAreExported_inGoogModule() {
testSame("goog.module('test'); var Foo = function() {var testA = function() {}}");
test(
"goog.module('test'); var setUp = function() {}",
"goog.module('test'); var setUp = function() {}; google_exportSymbol('setUp',setUp)");
}

// https://github.com/google/closure-compiler/issues/2563
@Test
public void testFunctionExpressionsInAssignAreExported() {
Expand All @@ -141,6 +157,13 @@ public void testFunctionExpressionsInAssignAreExported() {
"testBar = function() {}; google_exportSymbol('testBar',testBar)");
}

@Test
public void testFunctionExpressionsInAssignAreExported_inGoogModule() {
test(
"goog.module('test'); testBar = function() {};",
"goog.module('test'); testBar = function() {}; google_exportSymbol('testBar',testBar)");
}

@Test
public void testFunctionExpressionsByLetAreExported() {
testSame("let Foo = function() {var testA = function() {}}");
Expand Down Expand Up @@ -237,6 +260,62 @@ public void testExportTestSuite() {
"goog.testing.testSuite({'a': function() {}, 'b': function() {}});");
}

@Test
public void testExportTestSuite_whereGoogIsDefined() {
testSame("var goog = {}; goog.testing.testSuite({'a': function() {}, 'b': function() {}});");
test(
"var goog = {}; goog.testing.testSuite({a: function() {}, b: function() {}});",
"var goog = {}; goog.testing.testSuite({'a': function() {}, 'b': function() {}});");
}

@Test
public void testExportTestSuite_inGoogModule() {
test(
lines(
"goog.module('my.test');",
"const testSuite = goog.require('goog.testing.testSuite');",
"testSuite({a: function() {}, b: function() {}});"),
lines(
"goog.module('my.test');",
"const testSuite = goog.require('goog.testing.testSuite');",
"testSuite({'a': function() {}, 'b': function() {}});"));
}

@Test
public void testNonGoogTestingTestSuiteNotRewrittenInModule() {
testSame(
lines(
"goog.module('my.test');", //
"const testSuite = goog.require('some.other.testSuite');",
"testSuite({a: function() {}, b: function() {}});"));
}

@Test
public void testExportTestSuite_differentNameInGoogModule() {
test(
lines(
"goog.module('my.test');",
"const someRandomName = goog.require('goog.testing.testSuite');",
"someRandomName({a: function() {}, b: function() {}});"),
lines(
"goog.module('my.test');",
"const someRandomName = goog.require('goog.testing.testSuite');",
"someRandomName({'a': function() {}, 'b': function() {}});"));
}

@Test
public void testExportTestSuite_inEsModule() {
test(
lines(
"const testSuite = goog.require('goog.testing.testSuite');",
"testSuite({a: function() {}, b: function() {}});",
"export {};"), // Add `export {}` to make this an ES module.
lines(
"const testSuite = goog.require('goog.testing.testSuite');",
"testSuite({'a': function() {}, 'b': function() {}});",
"export {};"));
}

@Test
public void testMemberDefInObjLitInTestSuite_becomesStringKey() {
test(
Expand Down Expand Up @@ -279,6 +358,17 @@ public void testEs6Class_testMethod() {
+ "goog.testing.testSuite(new MyTest());");
}

@Test
public void testEs6Class_testMethod_inGoogModule() {
test(
"goog.module('test'); class MyTest {testFoo() {}} goog.testing.testSuite(new MyTest());",
lines(
"goog.module('test');",
"class MyTest {testFoo() {}} ",
"google_exportProperty(MyTest.prototype, 'testFoo', MyTest.prototype.testFoo); ",
"goog.testing.testSuite(new MyTest());"));
}

@Test
public void testEs6Class_lifeCycleMethods() {
test(
Expand Down

0 comments on commit 352f9c8

Please sign in to comment.