From 7f0734e7c8f527a6affa1c44137a587b0771fdfe Mon Sep 17 00:00:00 2001 From: Ivan Kalinin Date: Mon, 9 Jan 2023 13:59:13 +0300 Subject: [PATCH] [engine] Align story execution timeout properties naming --- .../ROOT/pages/tests-configuration.adoc | 8 +- .../org/vividus/batch/BatchConfiguration.java | 31 ++++--- .../java/org/vividus/batch/BatchStorage.java | 36 ++++++-- .../resources/org/vividus/engine/spring.xml | 3 +- .../org/vividus/BatchedEmbedderTests.java | 4 +- .../org/vividus/batch/BatchStorageTests.java | 86 +++++++++++++++---- .../properties/defaults/default.properties | 3 +- .../deprecated/deprecated.properties | 1 + 8 files changed, 131 insertions(+), 41 deletions(-) diff --git a/docs/modules/ROOT/pages/tests-configuration.adoc b/docs/modules/ROOT/pages/tests-configuration.adoc index 9c07fc2664..a4bbd5345c 100644 --- a/docs/modules/ROOT/pages/tests-configuration.adoc +++ b/docs/modules/ROOT/pages/tests-configuration.adoc @@ -72,9 +72,9 @@ NOTE: The properties marked with *bold* are mandatory. |`` |If set the value overrides global setting `scenario.fail-fast`. -|`batch-.story-execution-timeout` +|`batch-.story.execution-timeout` |`PT3H` -|The max duration of the single story in the batch. +|The max duration of the single story in the batch in {iso-date-format-link} format. Overrides value specified via `story.execution-timeout`. |`batch.fail-fast` |`false` @@ -88,6 +88,10 @@ NOTE: The properties marked with *bold* are mandatory. |`false` |If set to `true` the scenario execution will be stopped after first failed assertion +|`story.execution-timeout` +|`PT3H` +|The max duration of the single story in {iso-date-format-link} format. Could be overriden via corresponging batch setting. + |`bdd.configuration.dry-run` |`false` |Enables dry-run execution mode (no actual steps will be executed, dynamic variables and xref:ROOT:glossary.adoc#_expression[expressions] won't be resolved). For example dry-run could be useful to debug what stroies will be executed with provided config. diff --git a/vividus-engine/src/main/java/org/vividus/batch/BatchConfiguration.java b/vividus-engine/src/main/java/org/vividus/batch/BatchConfiguration.java index 4af685d5ce..a1157bfb3c 100644 --- a/vividus-engine/src/main/java/org/vividus/batch/BatchConfiguration.java +++ b/vividus-engine/src/main/java/org/vividus/batch/BatchConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2022 the original author or authors. + * Copyright 2019-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,10 +33,9 @@ public class BatchConfiguration private String name; private Integer threads; private List metaFilters; - private Duration storyExecutionTimeout; private Boolean failFast; - private ScenarioExecutionConfiguration scenario; - private StoryExecutionConfiguration story; + private ScenarioExecutionConfiguration scenario = new ScenarioExecutionConfiguration(); + private StoryExecutionConfiguration story = new StoryExecutionConfiguration(); private Map variables = Map.of(); public String getResourceLocation() @@ -106,12 +105,12 @@ public void setMetaFilters(String metaFilters) public Duration getStoryExecutionTimeout() { - return storyExecutionTimeout; + return story.executionTimeout; } - public void setStoryExecutionTimeout(Duration storyExecutionTimeout) + public void overrideStoryExecutionTimeout(Duration storyExecutionTimeout) { - this.storyExecutionTimeout = storyExecutionTimeout; + story.executionTimeout = storyExecutionTimeout; } public Boolean isFailFast() @@ -126,7 +125,7 @@ public void setFailFast(Boolean failFast) public Boolean isFailScenarioFast() { - return scenario == null ? null : scenario.failFast; + return scenario.failFast; } public void setScenario(ScenarioExecutionConfiguration scenario) @@ -136,7 +135,7 @@ public void setScenario(ScenarioExecutionConfiguration scenario) public Boolean isFailStoryFast() { - return story == null ? null : story.failFast; + return story.failFast; } public void setStory(StoryExecutionConfiguration story) @@ -167,19 +166,25 @@ private List convertToList(String list) private static final class StoryExecutionConfiguration { - private boolean failFast; + private Boolean failFast; + private Duration executionTimeout; - private void setFailFast(boolean failFast) + private void setFailFast(Boolean failFast) { this.failFast = failFast; } + + private void setExecutionTimeout(Duration executionTimeout) + { + this.executionTimeout = executionTimeout; + } } private static final class ScenarioExecutionConfiguration { - private boolean failFast; + private Boolean failFast; - private void setFailFast(boolean failFast) + private void setFailFast(Boolean failFast) { this.failFast = failFast; } diff --git a/vividus-engine/src/main/java/org/vividus/batch/BatchStorage.java b/vividus-engine/src/main/java/org/vividus/batch/BatchStorage.java index f4155a7bfa..b4552c1ae5 100644 --- a/vividus-engine/src/main/java/org/vividus/batch/BatchStorage.java +++ b/vividus-engine/src/main/java/org/vividus/batch/BatchStorage.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2022 the original author or authors. + * Copyright 2019-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,26 +21,48 @@ import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Optional; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.vividus.util.property.IPropertyMapper; public class BatchStorage { + private static final Logger LOGGER = LoggerFactory.getLogger(BatchStorage.class); + private static final String BATCH = "batch-"; + private static final String DEPRECATION_MESSAGE = + "Property `bdd.story-execution-timeout` is deprecated and will be removed in" + + " VIVIDUS 0.7.0. Please use `story.execution-timeout` instead."; + private static final Duration DEFAULT_STORY_TIMEOUT = Duration.ofHours(3); private final Map batchConfigurations; - private final Duration defaultStoryExecutionTimeout; private final List defaultMetaFilters; private final boolean failFast; - public BatchStorage(IPropertyMapper propertyMapper, String defaultStoryExecutionTimeout, - List defaultMetaFilters, boolean failFast) throws IOException + public BatchStorage(IPropertyMapper propertyMapper, Duration defaultStoryExecutionTimeout, + String deprecatedDefaultStoryExecutionTimeout, List defaultMetaFilters, + boolean failFast) throws IOException { this.defaultMetaFilters = defaultMetaFilters; - this.defaultStoryExecutionTimeout = Duration.ofSeconds(Long.parseLong(defaultStoryExecutionTimeout)); + if (null != deprecatedDefaultStoryExecutionTimeout) + { + Validate.isTrue(defaultStoryExecutionTimeout == null, + "Conflicting properties are found: `bdd.story-execution-timeout` and `story.execution-timeout`. " + + DEPRECATION_MESSAGE); + this.defaultStoryExecutionTimeout = + Duration.ofSeconds(Long.parseLong(deprecatedDefaultStoryExecutionTimeout)); + LOGGER.warn(DEPRECATION_MESSAGE); + } + else + { + this.defaultStoryExecutionTimeout = Optional.ofNullable(defaultStoryExecutionTimeout) + .orElse(DEFAULT_STORY_TIMEOUT); + } this.failFast = failFast; batchConfigurations = readFromProperties(propertyMapper, BATCH, BatchConfiguration.class); @@ -58,7 +80,7 @@ public BatchStorage(IPropertyMapper propertyMapper, String defaultStoryExecution } if (batchConfiguration.getStoryExecutionTimeout() == null) { - batchConfiguration.setStoryExecutionTimeout(this.defaultStoryExecutionTimeout); + batchConfiguration.overrideStoryExecutionTimeout(this.defaultStoryExecutionTimeout); } if (batchConfiguration.isFailFast() == null) { @@ -88,7 +110,7 @@ public BatchConfiguration getBatchConfiguration(String batchKey) return batchConfigurations.computeIfAbsent(batchKey, b -> { BatchConfiguration config = new BatchConfiguration(); config.setName(batchKey); - config.setStoryExecutionTimeout(defaultStoryExecutionTimeout); + config.overrideStoryExecutionTimeout(defaultStoryExecutionTimeout); config.setMetaFilters(defaultMetaFilters); config.setFailFast(failFast); return config; diff --git a/vividus-engine/src/main/resources/org/vividus/engine/spring.xml b/vividus-engine/src/main/resources/org/vividus/engine/spring.xml index 14fec4ab6f..0fbd0b5026 100644 --- a/vividus-engine/src/main/resources/org/vividus/engine/spring.xml +++ b/vividus-engine/src/main/resources/org/vividus/engine/spring.xml @@ -12,7 +12,8 @@ - + + diff --git a/vividus-engine/src/test/java/org/vividus/BatchedEmbedderTests.java b/vividus-engine/src/test/java/org/vividus/BatchedEmbedderTests.java index 6886430f5c..5dddefc764 100644 --- a/vividus-engine/src/test/java/org/vividus/BatchedEmbedderTests.java +++ b/vividus-engine/src/test/java/org/vividus/BatchedEmbedderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2022 the original author or authors. + * Copyright 2019-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -270,7 +270,7 @@ private void mockBatchConfiguration(boolean failFast) private void mockBatchConfiguration(boolean failFast, Boolean failStoryFast) { var batchConfiguration = spy(new BatchConfiguration()); - batchConfiguration.setStoryExecutionTimeout(Duration.ofHours(1)); + batchConfiguration.overrideStoryExecutionTimeout(Duration.ofHours(1)); batchConfiguration.setMetaFilters(META_FILTERS); batchConfiguration.setThreads(2); batchConfiguration.setFailFast(failFast); diff --git a/vividus-engine/src/test/java/org/vividus/batch/BatchStorageTests.java b/vividus-engine/src/test/java/org/vividus/batch/BatchStorageTests.java index df998d3eb9..7708486070 100644 --- a/vividus-engine/src/test/java/org/vividus/batch/BatchStorageTests.java +++ b/vividus-engine/src/test/java/org/vividus/batch/BatchStorageTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2022 the original author or authors. + * Copyright 2019-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.vividus.batch; +import static com.github.valfirst.slf4jtest.LoggingEvent.warn; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -35,13 +36,17 @@ import java.util.stream.Collectors; import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import com.github.valfirst.slf4jtest.TestLogger; +import com.github.valfirst.slf4jtest.TestLoggerFactory; +import com.github.valfirst.slf4jtest.TestLoggerFactoryExtension; import org.hamcrest.Matchers; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.vividus.util.property.PropertyMapper; import org.vividus.util.property.PropertyParser; +@ExtendWith(TestLoggerFactoryExtension.class) class BatchStorageTests { private static final List BATCH_NUMBERS = List.of("1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11"); @@ -49,10 +54,11 @@ class BatchStorageTests private static final List BATCH_KEYS = BATCH_NUMBERS.stream().map(n -> BATCH + n).collect( Collectors.toList()); + private static final Duration EXPECTED_DURATION = Duration.ofSeconds(300); private static final String DEFAULT_RESOURCE_LOCATION = ""; private static final String TRUE = "true"; + private static final String DOT = "."; - private static final long DEFAULT_TIMEOUT = 300; private static final List DEFAULT_META_FILTERS = List.of("groovy: !skip"); private static final String BATCH_2_NAME = "second batch name"; @@ -62,10 +68,11 @@ class BatchStorageTests private static final String VALUE_1 = "value1"; private static final String VALUE_2 = "value2"; + private static final TestLogger LOGGER = TestLoggerFactory.getTestLogger(BatchStorage.class); + private BatchStorage batchStorage; - @BeforeEach - void beforeEach() throws IOException + void initializeBatchStorage() throws IOException { var batchConfigurations = new HashMap(); addBatchConfiguration(batchConfigurations, 1); @@ -95,7 +102,7 @@ private void createBatchStorage(Map batchConfigurations) throws batchConfigurations.put("batch-2.story.fail-fast", TRUE); batchConfigurations.put("batch-2.name", BATCH_2_NAME); batchConfigurations.put("batch-2.threads", Integer.toString(BATCH_2_THREADS)); - batchConfigurations.put("batch-2.story-execution-timeout", BATCH_2_TIMEOUT.toString()); + batchConfigurations.put("batch-2.story.execution-timeout", BATCH_2_TIMEOUT.toString()); batchConfigurations.put("batch-2.meta-filters", BATCH_2_META_FILTERS); batchConfigurations.put("batch-2.fail-fast", TRUE); batchConfigurations.put("batch-2.variables.key1", VALUE_1); @@ -103,13 +110,14 @@ private void createBatchStorage(Map batchConfigurations) throws var propertyParser = mock(PropertyParser.class); when(propertyParser.getPropertiesByPrefix(BATCH)).thenReturn(batchConfigurations); - var propertyMapper = new PropertyMapper(".", PropertyNamingStrategies.KEBAB_CASE, propertyParser, Set.of()); - batchStorage = new BatchStorage(propertyMapper, Long.toString(DEFAULT_TIMEOUT), DEFAULT_META_FILTERS, false); + var propertyMapper = new PropertyMapper(DOT, PropertyNamingStrategies.KEBAB_CASE, propertyParser, Set.of()); + batchStorage = new BatchStorage(propertyMapper, EXPECTED_DURATION, null, DEFAULT_META_FILTERS, false); } @Test - void shouldGetBatchConfigurationByKey() + void shouldGetBatchConfigurationByKey() throws IOException { + initializeBatchStorage(); var batchConfiguration = batchStorage.getBatchConfiguration(BATCH_KEYS.get(0)); assertAll( () -> assertEquals(DEFAULT_RESOURCE_LOCATION, batchConfiguration.getResourceLocation()), @@ -119,21 +127,24 @@ void shouldGetBatchConfigurationByKey() } @Test - void shouldGetAllBatchConfigurations() + void shouldGetAllBatchConfigurations() throws IOException { + initializeBatchStorage(); Map allBatches = batchStorage.getBatchConfigurations(); assertThat(allBatches.keySet(), Matchers.contains(BATCH_KEYS.toArray())); } @Test - void shouldGetBatchConfigurationInitializedWithDefaultValues() + void shouldGetBatchConfigurationInitializedWithDefaultValues() throws IOException { + initializeBatchStorage(); assertDefaultBatchConfiguration(BATCH_KEYS.get(0)); } @Test - void shouldGetBatchConfigurationInitializedWithNonDefaultValues() + void shouldGetBatchConfigurationInitializedWithNonDefaultValues() throws IOException { + initializeBatchStorage(); var config = batchStorage.getBatchConfiguration(BATCH_KEYS.get(1)); assertAll( () -> assertEquals(BATCH_2_NAME, config.getName()), @@ -148,8 +159,9 @@ void shouldGetBatchConfigurationInitializedWithNonDefaultValues() } @Test - void shouldCreateNonExistentBatchConfiguration() + void shouldCreateNonExistentBatchConfiguration() throws IOException { + initializeBatchStorage(); assertDefaultBatchConfiguration("batch-100"); } @@ -159,7 +171,7 @@ private void assertDefaultBatchConfiguration(String batchKey) assertAll( () -> assertEquals(batchKey, config.getName()), () -> assertNull(config.getThreads()), - () -> assertEquals(Duration.ofSeconds(DEFAULT_TIMEOUT), config.getStoryExecutionTimeout()), + () -> assertEquals(EXPECTED_DURATION, config.getStoryExecutionTimeout()), () -> assertEquals(DEFAULT_META_FILTERS, config.getMetaFilters()), () -> assertFalse(config.isFailFast()), () -> assertNull(config.isFailStoryFast()), @@ -169,12 +181,56 @@ private void assertDefaultBatchConfiguration(String batchKey) } @Test - void shouldThrowErrorIfBatchResourceConfigurationDoesNotContainResourceLocation() + void shouldThrowErrorIfBatchResourceConfigurationDoesNotContainResourceLocation() throws IOException { + initializeBatchStorage(); var batchConfigurations = new HashMap(); batchConfigurations.put("batch-1.resource-include-patterns", "*.story"); var exception = assertThrows(IllegalArgumentException.class, () -> createBatchStorage(batchConfigurations)); assertEquals("'resource-location' is missing for batch-1", exception.getMessage()); } + + @Test + void shouldWarnAboutPlainSecondsUsage() throws IOException + { + var propertyParser = mock(PropertyParser.class); + when(propertyParser.getPropertiesByPrefix(BATCH)).thenReturn(Map.of("batch-999.resource-location", + DEFAULT_RESOURCE_LOCATION)); + + var propertyMapper = new PropertyMapper(DOT, PropertyNamingStrategies.KEBAB_CASE, propertyParser, Set.of()); + batchStorage = new BatchStorage(propertyMapper, null, "300", DEFAULT_META_FILTERS, false); + assertEquals(List.of(warn("Property `bdd.story-execution-timeout` is deprecated and will be removed" + + " in VIVIDUS 0.7.0. Please use `story.execution-timeout` instead.")), LOGGER.getLoggingEvents()); + var config = batchStorage.getBatchConfiguration("batch-999"); + assertEquals(EXPECTED_DURATION, config.getStoryExecutionTimeout()); + } + + @Test + void shouldThrowExceptionWhenBothPropertiesSpecified() + { + var propertyParser = mock(PropertyParser.class); + when(propertyParser.getPropertiesByPrefix(BATCH)).thenReturn(Map.of("batch-998.resource-location", + DEFAULT_RESOURCE_LOCATION)); + var duration = Duration.ofHours(111); + var propertyMapper = new PropertyMapper(DOT, PropertyNamingStrategies.KEBAB_CASE, propertyParser, Set.of()); + var iae = assertThrows(IllegalArgumentException.class, + () -> new BatchStorage(propertyMapper, duration, "202", DEFAULT_META_FILTERS, false)); + assertEquals(iae.getMessage(), "Conflicting properties are found: `bdd.story-execution-timeout`" + + " and `story.execution-timeout`. Property `bdd.story-execution-timeout` is deprecated and will be" + + " removed in VIVIDUS 0.7.0. Please use `story.execution-timeout` instead."); + } + + @Test + void shouldUseDefaultTimeout() throws IOException + { + var propertyParser = mock(PropertyParser.class); + when(propertyParser.getPropertiesByPrefix(BATCH)).thenReturn(Map.of("batch-997.resource-location", + DEFAULT_RESOURCE_LOCATION)); + + var propertyMapper = new PropertyMapper(DOT, PropertyNamingStrategies.KEBAB_CASE, propertyParser, Set.of()); + batchStorage = new BatchStorage(propertyMapper, null, null, DEFAULT_META_FILTERS, false); + var config = batchStorage.getBatchConfiguration("batch-997"); + assertEquals(Duration.ofHours(3), config.getStoryExecutionTimeout()); + } } diff --git a/vividus/src/main/resources/properties/defaults/default.properties b/vividus/src/main/resources/properties/defaults/default.properties index 5d1d42e2a2..5745502ec2 100644 --- a/vividus/src/main/resources/properties/defaults/default.properties +++ b/vividus/src/main/resources/properties/defaults/default.properties @@ -3,7 +3,8 @@ # bdd.all-meta-filters=+testType UI +regression -skip --- All tests with '@testType UI' marked as 'regression' and not marked as 'skip' bdd.all-meta-filters=groovy: !skip && (${bdd.meta-filters}) bdd.meta-filters=true -bdd.story-execution-timeout=10800 +# The property default value currently specified in org.vividus.batch.BatchStorage in order to support conflicting properties check. +# story.execution-timeout=PT3H bdd.cache-examples-table=false batch.fail-fast=false diff --git a/vividus/src/main/resources/properties/deprecated/deprecated.properties b/vividus/src/main/resources/properties/deprecated/deprecated.properties index 5d4c121ad9..42e68b76e8 100644 --- a/vividus/src/main/resources/properties/deprecated/deprecated.properties +++ b/vividus/src/main/resources/properties/deprecated/deprecated.properties @@ -7,3 +7,4 @@ bdd\\.variables\\.global\\.(.+)=variables.$1 bdd\\.variables\\.batch-(\\d+)\\.(.+)=batch-$1.variables.$2 bdd.configuration.composite-paths=engine.composite-paths bdd.configuration.all-composite-paths=bdd.configuration.composite-paths +batch-(\\d+)\\.story-execution-timeout=batch-$1.story.execution-timeout