Skip to content

Commit

Permalink
Make bazel test suites fail without a real test, by no longer adding …
Browse files Browse the repository at this point in the history
…an empty test in the TestSuiteBuilder.

The current scenario can be bug-prone since changes to the TestSuiteBuilder code may bypass all tests, and in the absence of test failures, our tests will signal success.

RELNOTES: Make it mandatory for Java test suites in bazel codebase, to contain at least one test.

--
PiperOrigin-RevId: 146919833
MOS_MIGRATED_REVID=146919833
  • Loading branch information
kush-c authored and kchodorow committed Feb 8, 2017
1 parent 777f3af commit b1f3302
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
6 changes: 3 additions & 3 deletions src/test/java/com/google/devtools/build/lib/AllTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib;

import com.google.devtools.build.lib.testutil.BlazeTestSuiteBuilder;
import com.google.devtools.build.lib.testutil.BazelTestSuiteBuilder;
import com.google.devtools.build.lib.testutil.CustomSuite;

import org.junit.runner.RunWith;
Expand All @@ -24,11 +24,11 @@
* General test suite with defaults suitable for most of our tests.
*/
@RunWith(CustomSuite.class)
public class AllTests extends BlazeTestSuiteBuilder {
public class AllTests extends BazelTestSuiteBuilder {
public static Set<Class<?>> suite() {
return new AllTests()
.getBuilder()
.matchClasses(BlazeTestSuiteBuilder.TEST_SUPPORTS_CURRENT_OS)
.matchClasses(BazelTestSuiteBuilder.TEST_SUPPORTS_CURRENT_OS)
.create();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
* A base class for constructing test suites by searching the classpath for
* tests, possibly restricted to a predicate.
*/
public class BlazeTestSuiteBuilder {
public class BazelTestSuiteBuilder {

/**
* @return a TestSuiteBuilder configured for Blaze.
* @return a TestSuiteBuilder configured for Bazel.
*/
protected TestSuiteBuilder getBuilder() {
return new TestSuiteBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,20 @@ public final class TestSuiteBuilder {

private Set<Class<?>> testClasses = Sets.newTreeSet(new TestClassNameComparator());
private Predicate<Class<?>> matchClassPredicate = Predicates.alwaysTrue();
private final boolean tolerateEmptyTestSuites;

public TestSuiteBuilder() {
tolerateEmptyTestSuites = false;
}

/**
* @param tolerateEmptyTestSuites set this to true to add an empty test which passes to the suite.
* Its better for Test Suites to fail when they create an empty set of classes to test, so new
* suites should avoid setting this to true.
*/
public TestSuiteBuilder(boolean tolerateEmptyTestSuites) {
this.tolerateEmptyTestSuites = tolerateEmptyTestSuites;
}

/**
* Adds the tests found (directly) in class {@code c} to the set of tests
Expand Down Expand Up @@ -84,7 +98,7 @@ public Set<Class<?>> create() {
for (Class<?> testClass : Iterables.filter(testClasses, matchClassPredicate)) {
result.add(testClass);
}
if (result.isEmpty()) {
if (tolerateEmptyTestSuites && result.isEmpty()) {
// We have some cases where the resulting test suite is empty, which some of our test
// infrastructure treats as an error.
result.add(TautologyTest.class);
Expand Down

0 comments on commit b1f3302

Please sign in to comment.