From 4b2b7c873cbe676545a344e9784d20377db09866 Mon Sep 17 00:00:00 2001 From: Aliaksei Boole Date: Mon, 24 Aug 2020 22:24:36 +0300 Subject: [PATCH] Add retires feature to serenity client (#119) * Add retires feature to client * Fix after code review: * Change method name from `isFailAdded` to `isFailPresent` * Add NumberFormatException handling to `retriesCount` method * Write more useful readme Co-authored-by: Aliaksei Boole --- README.md | 8 +- .../reportportal/ReportIntegrationConfig.java | 15 +++- .../reportportal/StartEventBuilder.java | 5 ++ .../invictum/reportportal/SuiteStorage.java | 41 +++++++++- .../github/invictum/reportportal/Utils.java | 3 + .../reportportal/recorder/Regular.java | 46 ++++++++--- .../reportportal/recorder/TestRecorder.java | 3 +- .../ReportIntegrationConfigTest.java | 49 ++++++++---- .../reportportal/StartEventBuilderTest.java | 10 +++ .../reportportal/recorder/RegularTest.java | 79 ++++++++++++++++++- 10 files changed, 226 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 8b65a1b..2c7e326 100644 --- a/README.md +++ b/README.md @@ -262,7 +262,13 @@ Both mentioned options are required before test mechanism start and must be spec mvn clean verify -Dserenity.rp.communication.dir=../sync-dir -Dserenity.rp.modules.count=2 ``` -With merge feature activation each submodule still produce separate launch on execution phase, but they will be merged into one at the end of all tests execution. +With merge feature activation each submodule still produce separate launch on execution phase, but they will be merged into one at the end of all tests execution. + +#### Test retries + +Report portal have the feature to show [test retries](https://github.com/reportportal/documentation/blob/master/src/md/src/DevGuides/retries.md). +Serenity RP client will report all retries automatically if you are use maven with *failsafe/surefire* plugin, *junit4* and add `failsafe.rerunFailingTestsCount` or `surefire.rerunFailingTestsCount` property to your test execution. +The feature work 100% only with this configuration. #### Other settings diff --git a/src/main/java/com/github/invictum/reportportal/ReportIntegrationConfig.java b/src/main/java/com/github/invictum/reportportal/ReportIntegrationConfig.java index 9a4919c..4fb821c 100644 --- a/src/main/java/com/github/invictum/reportportal/ReportIntegrationConfig.java +++ b/src/main/java/com/github/invictum/reportportal/ReportIntegrationConfig.java @@ -11,8 +11,10 @@ */ public class ReportIntegrationConfig { - static final String COMMUNICATION_DIR_KEY = "serenity.rp.communication.dir"; - static final String MODULES_COUNT_KEY = "serenity.rp.modules.count"; + public static final String COMMUNICATION_DIR_KEY = "serenity.rp.communication.dir"; + public static final String MODULES_COUNT_KEY = "serenity.rp.modules.count"; + public static final String FAILSAFE_RERUN_KEY = "failsafe.rerunFailingTestsCount"; + public static final String SUREFIRE_RERUN_KEY = "surefire.rerunFailingTestsCount"; private static volatile ReportIntegrationConfig instance; private LogsPreset preset = LogsPreset.DEFAULT; @@ -80,6 +82,15 @@ public String communicationDirectory() { return System.getProperty(COMMUNICATION_DIR_KEY); } + public int retriesCount() { + String value = System.getProperty(FAILSAFE_RERUN_KEY, System.getProperty(SUREFIRE_RERUN_KEY)); + try { + return value == null ? 0 : Integer.parseInt(value); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Wrong retires count", e); + } + } + public int modulesQuantity() { String value = System.getProperty(MODULES_COUNT_KEY); return value == null ? 0 : Integer.parseInt(value); diff --git a/src/main/java/com/github/invictum/reportportal/StartEventBuilder.java b/src/main/java/com/github/invictum/reportportal/StartEventBuilder.java index 1ff160c..bd0af7c 100644 --- a/src/main/java/com/github/invictum/reportportal/StartEventBuilder.java +++ b/src/main/java/com/github/invictum/reportportal/StartEventBuilder.java @@ -43,6 +43,11 @@ public StartEventBuilder withDescription(String description) { return this; } + public StartEventBuilder withRetry() { + startEvent.setRetry(true); + return this; + } + public StartEventBuilder withParameters(DataTable.RowValueAccessor data) { List parameters = data.toStringMap().entrySet().stream().map(param -> { ParameterResource parameter = new ParameterResource(); diff --git a/src/main/java/com/github/invictum/reportportal/SuiteStorage.java b/src/main/java/com/github/invictum/reportportal/SuiteStorage.java index da71fb3..4fe7e6c 100644 --- a/src/main/java/com/github/invictum/reportportal/SuiteStorage.java +++ b/src/main/java/com/github/invictum/reportportal/SuiteStorage.java @@ -2,6 +2,7 @@ import io.reactivex.Maybe; +import java.util.HashSet; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; @@ -48,16 +49,54 @@ public void suiteFinisher(String id, Runnable finisher) { */ public void finalizeActive() { suites.forEachKey(Runtime.getRuntime().availableProcessors(), id -> { - SuiteMetadata meta = suites.remove(id); + SuiteMetadata meta = suites.get(id); + if (meta.failedTests.isEmpty()) { + suites.remove(id); + } meta.finisher.run(); }); } + /** + * Register id of failed test for specific suite by id. Should be called if test failed first time. + * Part of retries logic. + * + * @param suiteId id of suite where fail test was detected + * @param testId id of failed test + */ + public void addFail(String suiteId, String testId) { + SuiteMetadata meta = suites.get(suiteId); + meta.failedTests.add(testId); + } + + /** + * This method need for check is test is retry. Part of retries logic. + * + * @param suiteId id of suite where fail test was detected + * @param testId id of failed test + */ + public boolean isFailPresent(String suiteId, String testId) { + SuiteMetadata meta = suites.get(suiteId); + return meta.failedTests.contains(testId); + } + + /** + * Should be called if failed test start passed after retry. Part of retries logic. + * + * @param suiteId id of suite where fail test was detected + * @param testId id of failed test + */ + public void removeFail(String suiteId, String testId) { + SuiteMetadata meta = suites.get(suiteId); + meta.failedTests.remove(testId); + } + /** * Node class that holds suite metadata */ private static class SuiteMetadata { private Maybe id; private Runnable finisher; + private final HashSet failedTests = new HashSet<>(); } } diff --git a/src/main/java/com/github/invictum/reportportal/Utils.java b/src/main/java/com/github/invictum/reportportal/Utils.java index 054b12f..380a545 100644 --- a/src/main/java/com/github/invictum/reportportal/Utils.java +++ b/src/main/java/com/github/invictum/reportportal/Utils.java @@ -10,6 +10,9 @@ public class Utils { + private Utils() { + } + /** * Finds log level based on step result status. * diff --git a/src/main/java/com/github/invictum/reportportal/recorder/Regular.java b/src/main/java/com/github/invictum/reportportal/recorder/Regular.java index 9759c3f..904806c 100644 --- a/src/main/java/com/github/invictum/reportportal/recorder/Regular.java +++ b/src/main/java/com/github/invictum/reportportal/recorder/Regular.java @@ -10,6 +10,7 @@ import com.google.inject.Inject; import io.reactivex.Maybe; import net.thucydides.core.model.TestOutcome; +import net.thucydides.core.model.TestResult; import java.util.Collection; @@ -17,6 +18,7 @@ * Common test recorder suitable for most cases */ public class Regular extends TestRecorder { + static final int RETRIES_COUNT = ReportIntegrationConfig.get().retriesCount(); @Inject public Regular(SuiteStorage suiteStorage, Launch launch, LogUnitsHolder holder) { @@ -25,34 +27,42 @@ public Regular(SuiteStorage suiteStorage, Launch launch, LogUnitsHolder holder) @Override public void record(TestOutcome out) { - StartEventBuilder startEventBuilder = new StartEventBuilder(ItemType.TEST) + StartEventBuilder suiteEventBuilder = new StartEventBuilder(ItemType.TEST) .withName(out.getUserStory().getDisplayName()) .withStartTime(out.getStartTime()) .withDescription(out.getUserStory().getNarrative()); NarrativeExtractor extractor = new NarrativeExtractor(out, ReportIntegrationConfig.get().formatter()); - extractor.extract().ifPresent(startEventBuilder::withDescription); - StartTestItemRQ startSuite = startEventBuilder.build(); - Maybe id = suiteStorage.start(out.getUserStory().getId(), () -> launch.startTestItem(startSuite)); - StartEventBuilder builder = new StartEventBuilder(ItemType.STEP); - builder.withName(out.getName()).withStartTime(out.getStartTime()).withTags(out.getTags()); + extractor.extract().ifPresent(suiteEventBuilder::withDescription); + StartTestItemRQ startSuite = suiteEventBuilder.build(); + Maybe suiteId = suiteStorage.start(out.getUserStory().getId(), () -> launch.startTestItem(startSuite)); + // Start test + StartEventBuilder testEventBuilder = new StartEventBuilder(ItemType.STEP) + .withName(out.getName()) + .withStartTime(out.getStartTime()) + .withTags(out.getTags()); + if (RETRIES_COUNT > 0) { + processRetries(out, testEventBuilder); + } if (out.isDataDriven()) { - builder.withParameters(out.getDataTable().row(0)); + testEventBuilder.withParameters(out.getDataTable().row(0)); } - Maybe testId = launch.startTestItem(id, builder.build()); + Maybe testId = launch.startTestItem(suiteId, testEventBuilder.build()); // Steps proceedSteps(testId, out.getTestSteps()); // failed assertions in test itself recordNonStepFailure(out); + // Stop test FinishTestItemRQ finishTest = new FinishEventBuilder() .withStatus(Status.mapTo(out.getResult())) .withEndTime(out.getStartTime(), out.getDuration()) .build(); launch.finishTestItem(testId, finishTest); + // Finish suite FinishTestItemRQ finishSuite = new FinishEventBuilder() .withStatus(Status.PASSED) .withEndTime(out.getStartTime(), out.getDuration()) .build(); - suiteStorage.suiteFinisher(out.getUserStory().getId(), () -> launch.finishTestItem(id, finishSuite)); + suiteStorage.suiteFinisher(out.getUserStory().getId(), () -> launch.finishTestItem(suiteId, finishSuite)); } private void recordNonStepFailure(TestOutcome out) { @@ -62,4 +72,22 @@ private void recordNonStepFailure(TestOutcome out) { return log; })); } + + private boolean isTestFailed(TestOutcome out) { + TestResult testResult = out.getResult(); + return testResult == TestResult.ERROR || testResult == TestResult.FAILURE; + } + + private void processRetries(TestOutcome out, StartEventBuilder builder) { + String testId = out.getId(); + String suiteId = out.getUserStory().getId(); + if (suiteStorage.isFailPresent(suiteId, testId)) { + builder.withRetry(); + if (!isTestFailed(out)) { + suiteStorage.removeFail(suiteId, testId); + } + } else if (isTestFailed(out)) { + suiteStorage.addFail(suiteId, testId); + } + } } diff --git a/src/main/java/com/github/invictum/reportportal/recorder/TestRecorder.java b/src/main/java/com/github/invictum/reportportal/recorder/TestRecorder.java index 9da033b..3ef6e46 100644 --- a/src/main/java/com/github/invictum/reportportal/recorder/TestRecorder.java +++ b/src/main/java/com/github/invictum/reportportal/recorder/TestRecorder.java @@ -16,7 +16,6 @@ * Defines steps to use to record particular test in context of suite */ public abstract class TestRecorder { - protected SuiteStorage suiteStorage; protected Launch launch; protected LogUnitsHolder holder; @@ -27,7 +26,7 @@ public TestRecorder(SuiteStorage suiteStorage, Launch launch, LogUnitsHolder hol this.holder = holder; } - abstract public void record(TestOutcome out); + public abstract void record(TestOutcome out); /** * Factory method to discover suitable {@link TestRecorder} approach diff --git a/src/test/java/com/github/invictum/reportportal/ReportIntegrationConfigTest.java b/src/test/java/com/github/invictum/reportportal/ReportIntegrationConfigTest.java index 5ad2299..b9fd077 100644 --- a/src/test/java/com/github/invictum/reportportal/ReportIntegrationConfigTest.java +++ b/src/test/java/com/github/invictum/reportportal/ReportIntegrationConfigTest.java @@ -7,8 +7,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import static com.github.invictum.reportportal.ReportIntegrationConfig.COMMUNICATION_DIR_KEY; -import static com.github.invictum.reportportal.ReportIntegrationConfig.MODULES_COUNT_KEY; +import static com.github.invictum.reportportal.ReportIntegrationConfig.*; @RunWith(JUnit4.class) public class ReportIntegrationConfigTest { @@ -21,54 +20,54 @@ public void before() { } @Test - public void defaultPreset() { + public void defaultPresetTest() { Assert.assertEquals(LogsPreset.DEFAULT, config.preset()); } @Test - public void customPreset() { + public void customPresetTest() { Assert.assertEquals(LogsPreset.CUSTOM, config.usePreset(LogsPreset.CUSTOM).preset()); } @Test(expected = NullPointerException.class) - public void nullCustomPreset() { + public void nullCustomPresetTest() { config.usePreset(null); } @Test - public void communicationDirectoryNotDefined() { + public void communicationDirectoryNotDefinedTest() { System.clearProperty(COMMUNICATION_DIR_KEY); Assert.assertNull(config.communicationDirectory()); } @Test - public void communicationDirectory() { + public void communicationDirectoryTest() { System.setProperty(COMMUNICATION_DIR_KEY, "dir"); - Assert.assertEquals(config.communicationDirectory(), "dir"); + Assert.assertEquals("dir", config.communicationDirectory()); } @Test - public void modulesQuantityNotDefined() { + public void modulesQuantityNotDefinedTest() { System.clearProperty(MODULES_COUNT_KEY); Assert.assertEquals(0, config.modulesQuantity()); } @Test - public void modulesQuantity() { + public void modulesQuantityTest() { System.setProperty(MODULES_COUNT_KEY, "42"); Assert.assertEquals(42, config.modulesQuantity()); } @Test - public void defaultClassNarrativeFormatter() { + public void defaultClassNarrativeFormatterTest() { Narrative narrative = TestInstance.class.getAnnotation(Narrative.class); String actual = config.formatter().apply(narrative); Assert.assertEquals("line 1\nline 2", actual); } @Test - public void overrideClassNarrativeFormatter() { + public void overrideClassNarrativeFormatterTest() { config.useClassNarrativeFormatter(n -> n.text()[0]); Narrative narrative = TestInstance.class.getAnnotation(Narrative.class); String actual = config.formatter().apply(narrative); @@ -76,12 +75,34 @@ public void overrideClassNarrativeFormatter() { } @Test - public void defaultTruncateNames() { + public void defaultTruncateNamesTest() { Assert.assertFalse(config.truncateNames); } @Test - public void truncateNames() { + public void truncateNamesTest() { Assert.assertTrue(config.truncateNames(true).truncateNames); } + + + @Test + public void retriesCountFailSafeTest() { + System.clearProperty(SUREFIRE_RERUN_KEY); + System.setProperty(FAILSAFE_RERUN_KEY, "42"); + Assert.assertEquals(42, config.retriesCount()); + } + + @Test + public void retriesCountSurefireTest() { + System.clearProperty(FAILSAFE_RERUN_KEY); + System.setProperty(SUREFIRE_RERUN_KEY, "69"); + Assert.assertEquals(69, config.retriesCount()); + } + + @Test + public void retriesCountDefaultTest() { + System.clearProperty(FAILSAFE_RERUN_KEY); + System.clearProperty(SUREFIRE_RERUN_KEY); + Assert.assertEquals(0, config.retriesCount()); + } } diff --git a/src/test/java/com/github/invictum/reportportal/StartEventBuilderTest.java b/src/test/java/com/github/invictum/reportportal/StartEventBuilderTest.java index daff3ed..1e08579 100644 --- a/src/test/java/com/github/invictum/reportportal/StartEventBuilderTest.java +++ b/src/test/java/com/github/invictum/reportportal/StartEventBuilderTest.java @@ -120,4 +120,14 @@ public void withTagsTest() { .build(); Assert.assertEquals(Collections.singleton(new ItemAttributesRQ("type", "name")), event.getAttributes()); } + + @Test + public void withRetryTest() { + StartTestItemRQ event = new StartEventBuilder(ItemType.TEST) + .withStartTime(ZonedDateTime.now()) + .withName("name") + .withRetry() + .build(); + Assert.assertTrue(event.isRetry()); + } } diff --git a/src/test/java/com/github/invictum/reportportal/recorder/RegularTest.java b/src/test/java/com/github/invictum/reportportal/recorder/RegularTest.java index 1ae2820..d65ecef 100644 --- a/src/test/java/com/github/invictum/reportportal/recorder/RegularTest.java +++ b/src/test/java/com/github/invictum/reportportal/recorder/RegularTest.java @@ -5,6 +5,8 @@ import com.github.invictum.reportportal.SuiteStorage; import net.thucydides.core.model.Story; import net.thucydides.core.model.TestOutcome; +import net.thucydides.core.model.TestResult; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -12,6 +14,9 @@ import java.time.ZonedDateTime; +import static com.github.invictum.reportportal.ReportIntegrationConfig.FAILSAFE_RERUN_KEY; +import static com.github.invictum.reportportal.ReportIntegrationConfig.SUREFIRE_RERUN_KEY; + @RunWith(JUnit4.class) public class RegularTest { @@ -19,19 +24,85 @@ public class RegularTest { private Launch launchMock; private LogUnitsHolder logUnitsHolderMock; - @Test - public void recordTest() { + @Before + public void before() { suiteStorageMock = Mockito.mock(SuiteStorage.class); launchMock = Mockito.mock(Launch.class); logUnitsHolderMock = Mockito.mock(LogUnitsHolder.class); + } + + @Test + public void regularRecordTest() { + System.clearProperty(FAILSAFE_RERUN_KEY); + System.clearProperty(SUREFIRE_RERUN_KEY); + TestRecorder recorder = new Regular(suiteStorageMock, launchMock, logUnitsHolderMock); + TestOutcome testOutcome = Mockito.mock(TestOutcome.class); + Mockito.when(testOutcome.getUserStory()).thenReturn(Story.called("story").withNarrative("narrative")); + ZonedDateTime start = ZonedDateTime.now(); + Mockito.when(testOutcome.getStartTime()).thenReturn(start); + Mockito.when(testOutcome.getName()).thenReturn("Test name"); + recorder.record(testOutcome); + Mockito.verify(suiteStorageMock, + Mockito.times(1)).start( + Mockito.eq("story"), Mockito.any()); + Mockito.verify(suiteStorageMock, + Mockito.times(1)).suiteFinisher( + Mockito.eq("story"), Mockito.any()); + } + + @Test + public void retryRecordCallIsFailPresentTest() { + System.setProperty(SUREFIRE_RERUN_KEY, "2"); + TestRecorder recorder = new Regular(suiteStorageMock, launchMock, logUnitsHolderMock); + TestOutcome testOutcome = Mockito.mock(TestOutcome.class); + Mockito.when(testOutcome.getUserStory()).thenReturn(Story.called("story").withNarrative("narrative")); + Mockito.when(testOutcome.getId()).thenReturn("testId"); + ZonedDateTime start = ZonedDateTime.now(); + Mockito.when(testOutcome.getStartTime()).thenReturn(start); + Mockito.when(testOutcome.getName()).thenReturn("Test name"); + recorder.record(testOutcome); + Mockito.verify(suiteStorageMock, + Mockito.times(1)).isFailPresent( + Mockito.eq("story"), Mockito.eq("testId")); + } + + @Test + public void retryRecordStoreNewFailTest() { + System.setProperty(SUREFIRE_RERUN_KEY, "2"); TestRecorder recorder = new Regular(suiteStorageMock, launchMock, logUnitsHolderMock); TestOutcome testOutcome = Mockito.mock(TestOutcome.class); Mockito.when(testOutcome.getUserStory()).thenReturn(Story.called("story").withNarrative("narrative")); + Mockito.when(testOutcome.getId()).thenReturn("testId"); + Mockito.when(testOutcome.getResult()).thenReturn(TestResult.ERROR); ZonedDateTime start = ZonedDateTime.now(); Mockito.when(testOutcome.getStartTime()).thenReturn(start); Mockito.when(testOutcome.getName()).thenReturn("Test name"); + Mockito.when(suiteStorageMock.isFailPresent(Mockito.any(), Mockito.any())).thenReturn(false); + recorder.record(testOutcome); + Mockito.when(testOutcome.getResult()).thenReturn(TestResult.FAILURE); recorder.record(testOutcome); - Mockito.verify(suiteStorageMock, Mockito.times(1)).start(Mockito.eq("story"), Mockito.any()); - Mockito.verify(suiteStorageMock, Mockito.times(1)).suiteFinisher(Mockito.eq("story"), Mockito.any()); + Mockito.verify(suiteStorageMock, + Mockito.times(2)).addFail( + Mockito.eq("story"), Mockito.eq("testId")); } + + @Test + public void retryRecordClearSuiteIsRetryPassedTest() { + System.setProperty(FAILSAFE_RERUN_KEY, "1"); + TestRecorder recorder = new Regular(suiteStorageMock, launchMock, logUnitsHolderMock); + TestOutcome testOutcome = Mockito.mock(TestOutcome.class); + Mockito.when(testOutcome.getUserStory()).thenReturn(Story.called("story").withNarrative("narrative")); + Mockito.when(testOutcome.getId()).thenReturn("testId"); + Mockito.when(testOutcome.getResult()).thenReturn(TestResult.SUCCESS); + ZonedDateTime start = ZonedDateTime.now(); + Mockito.when(testOutcome.getStartTime()).thenReturn(start); + Mockito.when(testOutcome.getName()).thenReturn("Test name"); + Mockito.when(suiteStorageMock.isFailPresent(Mockito.any(), Mockito.any())).thenReturn(true); + recorder.record(testOutcome); + Mockito.verify(suiteStorageMock, + Mockito.times(1)).removeFail( + Mockito.eq("story"), Mockito.eq("testId")); + } + + }