From ba6fdb99f00c52f7496a7d6aa485e9a33663a185 Mon Sep 17 00:00:00 2001 From: Googler Date: Sat, 8 Dec 2018 06:29:58 -0800 Subject: [PATCH] Add disclaimer to test case summary output when all test cases pass but target does not. Revamped tests to be deterministic (previously used a random number generator to determine number of passed and failed test cases). RELNOTES: Adds a clarifying message to test case summary output when all test cases pass but the target fails. PiperOrigin-RevId: 224647102 --- .../runtime/TerminalTestResultNotifier.java | 14 +- .../TerminalTestResultNotifierTest.java | 271 +++++++++++++----- 2 files changed, 216 insertions(+), 69 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java index 234aba922f2c25..b5a865c0a153ed 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java @@ -254,9 +254,9 @@ private void addToErrorList( private void printStats(TestResultStats stats) { TestSummaryFormat testSummaryFormat = options.getOptions(ExecutionOptions.class).testSummary; - if ((testSummaryFormat == DETAILED) || (testSummaryFormat == TESTCASE)) { - Integer passCount = stats.totalTestCases - stats.totalFailedTestCases; - printer.printLn( + if (testSummaryFormat == DETAILED || testSummaryFormat == TESTCASE) { + int passCount = stats.totalTestCases - stats.totalFailedTestCases; + String message = String.format( "Test cases: finished with %s%d passing%s and %s%d failing%s out of %d test cases", passCount > 0 ? AnsiTerminalPrinter.Mode.INFO : "", @@ -265,7 +265,13 @@ private void printStats(TestResultStats stats) { stats.totalFailedTestCases > 0 ? AnsiTerminalPrinter.Mode.ERROR : "", stats.totalFailedTestCases, AnsiTerminalPrinter.Mode.DEFAULT, - stats.totalTestCases)); + stats.totalTestCases); + if (passCount > 0 && stats.totalFailedTestCases == 0 && stats.failedCount > 0) { + // It is possible for a target to fail even if all of its test cases pass. To avoid + // confusion, we append the following disclaimer. + message += "\n(however note that at least one target failed)"; + } + printer.printLn(message); } if (!optionCheckTestsUpToDate()) { diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java index 7ab653ba0e5178..e355c0f7229a16 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java @@ -14,12 +14,17 @@ package com.google.devtools.build.lib.runtime; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.TestStrategy.TestSummaryFormat; import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier.TestSummaryOptions; @@ -29,102 +34,238 @@ import com.google.devtools.build.lib.view.test.TestStatus.TestCase; import com.google.devtools.build.lib.view.test.TestStatus.TestCase.Status; import com.google.devtools.common.options.OptionsParsingResult; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; -import java.util.Random; +import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Mockito; +import org.mockito.ArgumentCaptor; -/** Tests {@link TerminalTestResultNotifier}. */ +/** Tests for {@link TerminalTestResultNotifier}. */ @RunWith(JUnit4.class) -public class TerminalTestResultNotifierTest { +public final class TerminalTestResultNotifierTest { - private final OptionsParsingResult optionsParsingResult = - Mockito.mock(OptionsParsingResult.class); - private final AnsiTerminalPrinter ansiTerminalPrinter = Mockito.mock(AnsiTerminalPrinter.class); - private final Random random = new Random(); + private static final String ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER = + "however note that at least one target failed"; - private void givenExecutionOption(TestSummaryFormat format) { - ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS; - executionOptions.testSummary = format; - when(optionsParsingResult.getOptions(ExecutionOptions.class)).thenReturn(executionOptions); + private final OptionsParsingResult optionsParsingResult = mock(OptionsParsingResult.class); + private final AnsiTerminalPrinter ansiTerminalPrinter = mock(AnsiTerminalPrinter.class); - TestSummaryOptions testSummaryOptions = new TestSummaryOptions(); - testSummaryOptions.verboseSummary = true; - when(optionsParsingResult.getOptions(TestSummaryOptions.class)).thenReturn(testSummaryOptions); + private BlazeTestStatus targetStatus; + private int numFailedTestCases; + private int numTotalTestCases; + private TestSummaryFormat testSummaryFormat; + + @Test + public void testCaseOption_allPass() throws Exception { + testSummaryFormat = TestSummaryFormat.TESTCASE; + numFailedTestCases = 0; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.PASSED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains(info("10 passing")); + assertThat(printed).contains("0 failing"); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString()); } - private void runTest(Boolean shouldPrintTestCaseSummary) throws Exception { - int numOfTotalTestCases = random.nextInt(10) + 1; - int numOfFailedCases = random.nextInt(numOfTotalTestCases); - int numOfSuccessfulTestCases = numOfTotalTestCases - numOfFailedCases; + @Test + public void testCaseOption_allPassButTargetFails() throws Exception { + testSummaryFormat = TestSummaryFormat.TESTCASE; + numFailedTestCases = 0; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; - TerminalTestResultNotifier terminalTestResultNotifier = new TerminalTestResultNotifier( - ansiTerminalPrinter, Path::getPathString, optionsParsingResult); + printTestCaseSummary(); - TestSummary testSummary = Mockito.mock(TestSummary.class); - when(testSummary.getTotalTestCases()).thenReturn(numOfTotalTestCases); - TestCase failedTestCase = TestCase.newBuilder().setStatus(Status.FAILED).build(); - ArrayList testCases = - new ArrayList<>(Collections.nCopies(numOfFailedCases, failedTestCase)); + String printed = getPrintedMessage(); + assertThat(printed).contains(info("10 passing")); + assertThat(printed).contains("0 failing"); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).contains(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString()); + } - Label labelA = Label.parseAbsolute("//foo/bar:baz", ImmutableMap.of()); - when(testSummary.getFailedTestCases()).thenReturn(testCases); - when(testSummary.getStatus()).thenReturn(BlazeTestStatus.FAILED); - when(testSummary.getLabel()).thenReturn(labelA); + @Test + public void testCaseOption_someFail() throws Exception { + testSummaryFormat = TestSummaryFormat.TESTCASE; + numFailedTestCases = 2; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; - HashSet testSummaries = new HashSet<>(); - testSummaries.add(testSummary); - terminalTestResultNotifier.notify(testSummaries, 1); + printTestCaseSummary(); - String summaryMessage = - String.format( - "Test cases: finished with %s%d passing%s and %s%d failing%s out of %d test cases", - numOfSuccessfulTestCases > 0 ? AnsiTerminalPrinter.Mode.INFO : "", - numOfSuccessfulTestCases, - AnsiTerminalPrinter.Mode.DEFAULT, - numOfFailedCases > 0 ? AnsiTerminalPrinter.Mode.ERROR : "", - numOfFailedCases, - AnsiTerminalPrinter.Mode.DEFAULT, - numOfTotalTestCases); + String printed = getPrintedMessage(); + assertThat(printed).contains(info("8 passing")); + assertThat(printed).contains(error("2 failing")); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + } - if (shouldPrintTestCaseSummary) { - verify(ansiTerminalPrinter).printLn(summaryMessage); - } else { - verify(ansiTerminalPrinter, never()).printLn(summaryMessage); - } + @Test + public void testCaseOption_allFail() throws Exception { + testSummaryFormat = TestSummaryFormat.TESTCASE; + numFailedTestCases = 10; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains("0 passing"); + assertThat(printed).contains(error("10 failing")); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.INFO.toString()); } @Test - public void testCasesDataVisibleInTestCaseOption() throws Exception { - givenExecutionOption(TestSummaryFormat.TESTCASE); - runTest(true); + public void detailedOption_allPass() throws Exception { + testSummaryFormat = TestSummaryFormat.DETAILED; + numFailedTestCases = 0; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.PASSED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains(info("10 passing")); + assertThat(printed).contains("0 failing"); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString()); } @Test - public void testCasesDataVisibleInDetailedOption() throws Exception { - givenExecutionOption(TestSummaryFormat.DETAILED); - runTest(true); + public void detailedOption_allPassButTargetFails() throws Exception { + testSummaryFormat = TestSummaryFormat.DETAILED; + numFailedTestCases = 0; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains(info("10 passing")); + assertThat(printed).contains("0 failing"); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).contains(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString()); } @Test - public void testCasesDataInVisibleInShortOption() throws Exception { - givenExecutionOption(TestSummaryFormat.SHORT); - runTest(false); + public void detailedOption_someFail() throws Exception { + testSummaryFormat = TestSummaryFormat.DETAILED; + numFailedTestCases = 2; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains(info("8 passing")); + assertThat(printed).contains(error("2 failing")); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); } @Test - public void testCasesDataInVisibleInTerseOption() throws Exception { - givenExecutionOption(TestSummaryFormat.TERSE); - runTest(false); + public void detailedOption_allFail() throws Exception { + testSummaryFormat = TestSummaryFormat.DETAILED; + numFailedTestCases = 10; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + String printed = getPrintedMessage(); + assertThat(printed).contains("0 passing"); + assertThat(printed).contains(error("10 failing")); + assertThat(printed).contains("out of 10 test cases"); + assertThat(printed).doesNotContain(ALL_TEST_CASES_PASSED_BUT_TARGET_FAILED_DISCLAIMER); + assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.INFO.toString()); } @Test - public void testCasesDataInVisibleInNoneOption() throws Exception { - givenExecutionOption(TestSummaryFormat.NONE); - runTest(false); + public void shortOption_noSummaryPrinted() throws Exception { + testSummaryFormat = TestSummaryFormat.SHORT; + numFailedTestCases = 2; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + verifyNoSummaryPrinted(); + } + + @Test + public void terseOption_noSummaryPrinted() throws Exception { + testSummaryFormat = TestSummaryFormat.TERSE; + numFailedTestCases = 2; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + verifyNoSummaryPrinted(); + } + + @Test + public void noneOption_noSummaryPrinted() throws Exception { + testSummaryFormat = TestSummaryFormat.NONE; + numFailedTestCases = 2; + numTotalTestCases = 10; + targetStatus = BlazeTestStatus.FAILED; + + printTestCaseSummary(); + + verifyNoSummaryPrinted(); + } + + private void printTestCaseSummary() throws LabelSyntaxException { + ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS; + executionOptions.testSummary = testSummaryFormat; + when(optionsParsingResult.getOptions(ExecutionOptions.class)).thenReturn(executionOptions); + + TestSummaryOptions testSummaryOptions = new TestSummaryOptions(); + testSummaryOptions.verboseSummary = true; + when(optionsParsingResult.getOptions(TestSummaryOptions.class)).thenReturn(testSummaryOptions); + + TestSummary testSummary = mock(TestSummary.class); + when(testSummary.getTotalTestCases()).thenReturn(numTotalTestCases); + TestCase failedTestCase = TestCase.newBuilder().setStatus(Status.FAILED).build(); + List failedTestCases = Collections.nCopies(numFailedTestCases, failedTestCase); + + Label labelA = Label.parseAbsolute("//foo/bar:baz", ImmutableMap.of()); + when(testSummary.getFailedTestCases()).thenReturn(failedTestCases); + when(testSummary.getStatus()).thenReturn(targetStatus); + when(testSummary.getLabel()).thenReturn(labelA); + + TerminalTestResultNotifier terminalTestResultNotifier = + new TerminalTestResultNotifier( + ansiTerminalPrinter, Path::getPathString, optionsParsingResult); + terminalTestResultNotifier.notify(ImmutableSet.of(testSummary), 1); + } + + private String getPrintedMessage() { + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + verify(ansiTerminalPrinter).printLn(messageCaptor.capture()); + return messageCaptor.getValue(); + } + + private void verifyNoSummaryPrinted() { + verify(ansiTerminalPrinter, never()).printLn(any()); + } + + private static String info(String message) { + return AnsiTerminalPrinter.Mode.INFO + message + AnsiTerminalPrinter.Mode.DEFAULT; + } + + private static String error(String message) { + return AnsiTerminalPrinter.Mode.ERROR + message + AnsiTerminalPrinter.Mode.DEFAULT; } }