From e2ba69ec03aa56d6eed67267e462438a84ed0aef Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Thu, 5 Dec 2019 15:16:19 -0800 Subject: [PATCH 1/6] Added TestMode Live, DoNotRecord annotation, and TestContextManager --- .../azure/core/test/InterceptorManager.java | 137 +++++++++++++----- .../java/com/azure/core/test/TestBase.java | 53 +++++-- .../azure/core/test/TestContextManager.java | 63 ++++++++ .../java/com/azure/core/test/TestMode.java | 4 + .../core/test/annotation/DoNotRecord.java | 32 ++++ .../core/test/annotation/package-info.java | 6 + .../core/test/utils/TestResourceNamer.java | 72 +++++---- .../core/test/InterceptorManagerTests.java | 57 ++++++++ .../core/test/TestContextManagerTests.java | 96 ++++++++++++ .../test/utils/TestResourceNamerTests.java | 113 +++++++++++++++ 10 files changed, 554 insertions(+), 79 deletions(-) create mode 100644 sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java create mode 100644 sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/DoNotRecord.java create mode 100644 sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/package-info.java create mode 100644 sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java create mode 100644 sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java create mode 100644 sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 5b07dd2051654..505e8aaabcd23 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -14,6 +14,7 @@ import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.net.URL; import java.util.HashMap; import java.util.Map; @@ -21,14 +22,16 @@ /** * A class that keeps track of network calls by either reading the data from an existing test session record or - * recording the network calls in memory. Test session records are saved or read from: - * "session-records/{@code testName}.json" + * recording the network calls in memory. Test session records are saved or read from: "session-records/{@code + * testName}.json" * * * * When the {@link InterceptorManager} is disposed, if the {@code testMode} is {@link TestMode#RECORD}, the network @@ -41,6 +44,8 @@ public class InterceptorManager implements AutoCloseable { private final Map textReplacementRules; private final String testName; private final TestMode testMode; + private final boolean allowedToReadRecordedValues; + private final boolean allowedToRecordValues; // Stores a map of all the HTTP properties in a session // A state machine ensuring a test is always reset before another one is setup @@ -60,44 +65,100 @@ public class InterceptorManager implements AutoCloseable { * * @param testName Name of the test session record. * @param testMode The {@link TestMode} for this interceptor. - * @throws IOException If {@code testMode} is {@link TestMode#PLAYBACK} and an existing test session record could - * not be located or the data could not be deserialized into an instance of {@link RecordedData}. + * @throws UncheckedIOException If {@code testMode} is {@link TestMode#PLAYBACK} and an existing test session record + * could not be located or the data could not be deserialized into an instance of {@link RecordedData}. * @throws NullPointerException If {@code testName} is {@code null}. + * @deprecated Use {@link #InterceptorManager(String, TestMode, boolean)} instead. */ - public InterceptorManager(String testName, TestMode testMode) throws IOException { + @Deprecated + public InterceptorManager(String testName, TestMode testMode) { + this(testName, testMode, false); + } + + /** + * Creates a new InterceptorManager that either replays test-session records or saves them. + * + * + * + * The test session records are persisted in the path: "session-records/{@code testName}.json" + * + * @param testName Name of the test session record. + * @param testMode The {@link TestMode} for this interceptor. + * @param doNotRecord Flag indicating whether network calls should be record or played back. + * @throws UncheckedIOException If {@code testMode} is {@link TestMode#PLAYBACK} and an existing test session record + * could not be located or the data could not be deserialized into an instance of {@link RecordedData}. + * @throws NullPointerException If {@code testName} is {@code null}. + */ + public InterceptorManager(String testName, TestMode testMode, boolean doNotRecord) { Objects.requireNonNull(testName, "'testName' cannot be null."); this.testName = testName; this.testMode = testMode; this.textReplacementRules = new HashMap<>(); + this.allowedToReadRecordedValues = (testMode == TestMode.PLAYBACK && !doNotRecord); + this.allowedToRecordValues = (testMode == TestMode.RECORD && !doNotRecord); + + if (allowedToReadRecordedValues) { + this.recordedData = readDataFromFile(); + } else if (allowedToRecordValues) { + this.recordedData = new RecordedData(); + } else { + this.recordedData = null; + } + } - this.recordedData = testMode == TestMode.PLAYBACK - ? readDataFromFile() - : new RecordedData(); + /** + * Creates a new InterceptorManager that replays test session records. It takes a set of {@code + * textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link + * NetworkCallRecord#getResponse()}. + * + * The test session records are read from: "session-records/{@code testName}.json" + * + * @param testName Name of the test session record. + * @param textReplacementRules A set of rules to replace text in {@link NetworkCallRecord#getResponse()} when + * playing back network calls. + * @throws UncheckedIOException An existing test session record could not be located or the data could not be + * deserialized into an instance of {@link RecordedData}. + * @throws NullPointerException If {@code testName} or {@code textReplacementRules} is {@code null}. + * @deprecated Use {@link #InterceptorManager(String, Map, boolean)} instead. + */ + @Deprecated + public InterceptorManager(String testName, Map textReplacementRules) { + this(testName, textReplacementRules, false); } /** - * Creates a new InterceptorManager that replays test session records. It takes a set of - * {@code textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a - * {@link NetworkCallRecord#getResponse()}. + * Creates a new InterceptorManager that replays test session records. It takes a set of {@code + * textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link + * NetworkCallRecord#getResponse()}. * * The test session records are read from: "session-records/{@code testName}.json" * * @param testName Name of the test session record. - * @param textReplacementRules A set of rules to replace text in {@link NetworkCallRecord#getResponse()} when playing - * back network calls. - * @throws IOException An existing test session record could not be located or the data could not be deserialized - * into an instance of {@link RecordedData}. + * @param textReplacementRules A set of rules to replace text in {@link NetworkCallRecord#getResponse()} when + * playing back network calls. + * @param doNotRecord Flag indicating whether network calls should be record or played back. + * @throws UncheckedIOException An existing test session record could not be located or the data could not be + * deserialized into an instance of {@link RecordedData}. * @throws NullPointerException If {@code testName} or {@code textReplacementRules} is {@code null}. */ - public InterceptorManager(String testName, Map textReplacementRules) throws IOException { + public InterceptorManager(String testName, Map textReplacementRules, boolean doNotRecord) { Objects.requireNonNull(testName, "'testName' cannot be null."); Objects.requireNonNull(textReplacementRules, "'textReplacementRules' cannot be null."); this.testName = testName; this.testMode = TestMode.PLAYBACK; + this.allowedToReadRecordedValues = !doNotRecord; + this.allowedToRecordValues = false; - this.recordedData = readDataFromFile(); + this.recordedData = allowedToReadRecordedValues ? readDataFromFile() : null; this.textReplacementRules = textReplacementRules; } @@ -120,7 +181,8 @@ public RecordedData getRecordedData() { } /** - * Gets a new HTTP pipeline policy that records network calls and its data is managed by {@link InterceptorManager}. + * Gets a new HTTP pipeline policy that records network calls and its data is managed by {@link + * InterceptorManager}. * * @return HttpPipelinePolicy to record network calls. */ @@ -145,27 +207,24 @@ public HttpClient getPlaybackClient() { */ @Override public void close() { - switch (testMode) { - case RECORD: - try { - writeDataToFile(); - } catch (IOException e) { - logger.error("Unable to write data to playback file.", e); - } - break; - case PLAYBACK: - // Do nothing - break; - default: - logger.error("==> Unknown AZURE_TEST_MODE: {}", testMode); - break; + if (allowedToRecordValues) { + try { + writeDataToFile(); + } catch (IOException e) { + logger.error("Unable to write data to playback file.", e); + } } } - private RecordedData readDataFromFile() throws IOException { + private RecordedData readDataFromFile() { File recordFile = getRecordFile(testName); ObjectMapper mapper = new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT); - return mapper.readValue(recordFile, RecordedData.class); + + try { + return mapper.readValue(recordFile, RecordedData.class); + } catch (IOException ex) { + throw logger.logExceptionAsWarning(new UncheckedIOException(ex)); + } } /* @@ -183,10 +242,11 @@ private File getRecordFile(String testName) { File playbackFile = new File(getRecordFolder(), testName + ".json"); if (!playbackFile.exists()) { - throw logger.logExceptionAsError(new RuntimeException(String.format( - "Missing playback file. File path: %s. ", playbackFile))); + throw logger.logExceptionAsError(new RuntimeException(String.format( + "Missing playback file. File path: %s. ", playbackFile.getPath()))); } - logger.info("==> Playback file path: " + playbackFile); + + logger.info("==> Playback file path: " + playbackFile.getPath()); return playbackFile; } @@ -218,7 +278,8 @@ private File createRecordFile(String testName) throws IOException { } /** - * Add text replacement rule (regex as key, the replacement text as value) into {@link InterceptorManager#textReplacementRules} + * Add text replacement rule (regex as key, the replacement text as value) into {@link + * InterceptorManager#textReplacementRules} * * @param regex the pattern to locate the position of replacement * @param replacement the replacement text diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java index e1d16afb4d100..ce2c2fc055cae 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java @@ -2,33 +2,35 @@ // Licensed under the MIT License. package com.azure.core.test; -import com.azure.core.util.Configuration; import com.azure.core.test.utils.TestResourceNamer; +import com.azure.core.util.Configuration; import com.azure.core.util.logging.ClientLogger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeAll; - -import java.io.IOException; -import java.lang.reflect.Method; -import java.util.Locale; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; +import java.io.UncheckedIOException; +import java.lang.reflect.Method; +import java.util.Locale; + /** * Base class for running live and playback tests using {@link InterceptorManager}. */ public abstract class TestBase implements BeforeEachCallback { // Environment variable name used to determine the TestMode. private static final String AZURE_TEST_MODE = "AZURE_TEST_MODE"; - private static TestMode testMode; + static TestMode testMode; private final ClientLogger logger = new ClientLogger(TestBase.class); protected InterceptorManager interceptorManager; protected TestResourceNamer testResourceNamer; + protected TestContextManager testContextManager; + private ExtensionContext extensionContext; /** @@ -53,16 +55,21 @@ public void beforeEach(ExtensionContext extensionContext) { */ @BeforeEach public void setupTest(TestInfo testInfo) { - final String testName = testInfo.getTestMethod().get().getName(); + final Method testMethod = testInfo.getTestMethod().get(); + this.testContextManager = new TestContextManager(testMethod); + testContextManager.verifyTestCanRunInTestMode(testMode); + + final String testName = testMethod.getName(); logger.info("Test Mode: {}, Name: {}", testMode, testName); try { - interceptorManager = new InterceptorManager(testName, testMode); - } catch (IOException e) { + interceptorManager = new InterceptorManager(testName, testMode, testContextManager.doNotRecordTest()); + } catch (UncheckedIOException e) { logger.error("Could not create interceptor for {}", testName, e); Assertions.fail(); } - testResourceNamer = new TestResourceNamer(testName, testMode, interceptorManager.getRecordedData()); + testResourceNamer = new TestResourceNamer(testName, testMode, testContextManager.doNotRecordTest(), + interceptorManager.getRecordedData()); beforeTest(); } @@ -73,8 +80,10 @@ public void setupTest(TestInfo testInfo) { */ @AfterEach public void teardownTest(TestInfo testInfo) { - afterTest(); - interceptorManager.close(); + if (testContextManager.wasTestRan()) { + afterTest(); + interceptorManager.close(); + } } /** @@ -133,4 +142,22 @@ private static TestMode initializeTestMode() { logger.info("Environment variable '{}' has not been set yet. Using 'Playback' mode.", AZURE_TEST_MODE); return TestMode.PLAYBACK; } + + /** + * Sleeps the test for the given amount of milliseconds if {@link TestMode} isn't {@link TestMode#PLAYBACK}. + * + * @param millis Number of milliseconds to sleep the test. + * @throws IllegalStateException If the sleep is interrupted. + */ + protected void sleepIfRunningAgainstService(long millis) { + if (testMode == TestMode.PLAYBACK) { + return; + } + + try { + Thread.sleep(millis); + } catch (InterruptedException ex) { + throw logger.logExceptionAsWarning(new IllegalStateException(ex)); + } + } } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java new file mode 100644 index 0000000000000..9b1ab4432e0e9 --- /dev/null +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.azure.core.test; + +import com.azure.core.test.annotation.DoNotRecord; + +import java.lang.reflect.Method; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * This class handles managing context about a test, such as custom testing annotations and verifying whether the test + * is capable of running. + */ +public class TestContextManager { + private final boolean doNotRecord; + private final boolean skipInPlayback; + private volatile boolean testRan; + + /** + * Constructs a {@link TestContextManager} based on the test method. + * + * @param testMethod Test method being ran. + */ + public TestContextManager(Method testMethod) { + DoNotRecord doNotRecordAnnotation = testMethod.getAnnotation(DoNotRecord.class); + if (doNotRecordAnnotation != null) { + this.doNotRecord = true; + this.skipInPlayback = doNotRecordAnnotation.skipInPlayback(); + } else { + this.doNotRecord = false; + this.skipInPlayback = false; + } + } + + /** + * Verifies whether the current test is allowed to run. + * + * @param testMode The {@link TestMode} tests are being ran in. + */ + public void verifyTestCanRunInTestMode(TestMode testMode) { + this.testRan = !(skipInPlayback && testMode == TestMode.PLAYBACK); + assumeTrue(testRan, "Test ddes not allow playback and was ran in 'TestMode.PLAYBACK'"); + } + + /** + * Returns whether the test should have its network calls recorded during a {@link TestMode#RECORD record} test run. + * + * @return Flag indicating whether to record test network calls. + */ + public boolean doNotRecordTest() { + return doNotRecord; + } + + /** + * Returns whether the current test was ran. + * + * @return Flag indicating whether the current test was ran. + */ + public boolean wasTestRan() { + return testRan; + } +} diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestMode.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestMode.java index 27b1224ff19f6..5d89b4de093eb 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestMode.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestMode.java @@ -10,6 +10,10 @@ public enum TestMode { * Record data from a live test. */ RECORD, + /** + * Run a live test without recording. + */ + LIVE, /** * Playback data from an existing test session. */ diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/DoNotRecord.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/DoNotRecord.java new file mode 100644 index 0000000000000..f2241c79de296 --- /dev/null +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/DoNotRecord.java @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.test.annotation; + +import com.azure.core.test.TestMode; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +/** + * Annotation given to some tests to indicate that network calls made during the test shouldn't be recorded. + * + *

+ * Pass {@code true} for {@link #skipInPlayback() skipInPlayback} to indicate that the test shouldn't run when tests are + * ran in {@link TestMode#PLAYBACK}. A common case for setting this to {@code true} is when the test has either + * sensitive content that cannot be redacted or calls into code that cannot be mocked. + */ +@Retention(RUNTIME) +@Target({METHOD}) +public @interface DoNotRecord { + + /** + * Returns whether the test will be ignored during a {@link TestMode#PLAYBACK playback} test run. + * + * @return Flag indicating if the test will be ignored during a playback test run. + */ + boolean skipInPlayback() default false; +} diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/package-info.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/package-info.java new file mode 100644 index 0000000000000..f114506b11efd --- /dev/null +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/annotation/package-info.java @@ -0,0 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +/** + * Package containing annotations test classes for Azure client libraries. + */ +package com.azure.core.test.annotation; diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java index 481d6be0607ba..31ca4488fd2fc 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java @@ -9,6 +9,8 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.Objects; +import java.util.function.Function; +import java.util.function.Supplier; /** * Provides random string names. If the test mode is {@link TestMode#PLAYBACK}, then names are fetched from @@ -16,21 +18,44 @@ * persisted to {@link RecordedData}. */ public class TestResourceNamer extends ResourceNamer { - private final TestMode testMode; private final RecordedData recordedData; + private final boolean allowedToReadRecordedValues; + private final boolean allowedToRecordValues; /** * Constructor of TestResourceNamer * + * @deprecated Use {@link #TestResourceNamer(String, TestMode, boolean, RecordedData)} instead. * @param name test name as prefix - * @param testMode the test mode {@link TestMode#PLAYBACK} or {@link TestMode#RECORD} + * @param testMode The {@link TestMode} which the test is running in. * @param recordedData the recorded data with list of network call */ + @Deprecated public TestResourceNamer(String name, TestMode testMode, RecordedData recordedData) { + this(name, testMode, false, recordedData); + } + + /** + * Constructor of TestResourceNamer + * + * @param name test name as prefix + * @param testMode The {@link TestMode} which the test is running in. + * @param doNotRecord Flag indicating whether values produced by this class should be recorded or played back. + * @param recordedData the recorded data with list of network call + * @throws NullPointerException If {@code testMode} isn't {@link TestMode#LIVE}, {@code doNotRecord} is + * {@code false}, and {@code recordedData} is {@code null}. + */ + public TestResourceNamer(String name, TestMode testMode, boolean doNotRecord, RecordedData recordedData) { super(name); - Objects.requireNonNull(recordedData, "'recordedData' cannot be null."); + + // Only need recordedData if the test is running in playback or record. + if (testMode != TestMode.LIVE && !doNotRecord) { + Objects.requireNonNull(recordedData, "'recordedData' cannot be null."); + } + this.recordedData = recordedData; - this.testMode = testMode; + this.allowedToReadRecordedValues = (testMode == TestMode.PLAYBACK && !doNotRecord); + this.allowedToRecordValues = (testMode == TestMode.RECORD && !doNotRecord); } /** @@ -42,13 +67,7 @@ public TestResourceNamer(String name, TestMode testMode, RecordedData recordedDa */ @Override public String randomName(String prefix, int maxLen) { - if (testMode == TestMode.PLAYBACK) { - return recordedData.removeVariable(); - } else { - String name = super.randomName(prefix, maxLen); - recordedData.addVariable(name); - return name; - } + return getValue(readValue -> readValue, () -> super.randomName(prefix, maxLen)); } /** @@ -58,13 +77,7 @@ public String randomName(String prefix, int maxLen) { */ @Override public String randomUuid() { - if (testMode == TestMode.PLAYBACK) { - return recordedData.removeVariable(); - } else { - String uuid = super.randomUuid(); - recordedData.addVariable(uuid); - return uuid; - } + return getValue(readValue -> readValue, super::randomUuid); } /** @@ -73,13 +86,7 @@ public String randomUuid() { * @return OffsetDateTime of UTC now. */ public OffsetDateTime now() { - if (testMode == TestMode.PLAYBACK) { - return OffsetDateTime.parse(recordedData.removeVariable()); - } else { - OffsetDateTime now = OffsetDateTime.now(ZoneOffset.UTC); - recordedData.addVariable(now.toString()); - return now; - } + return getValue(OffsetDateTime::parse, () -> OffsetDateTime.now(ZoneOffset.UTC)); } /** @@ -89,10 +96,19 @@ public OffsetDateTime now() { * @return the recorded value. */ public String recordValueFromConfig(String value) { - if (testMode == TestMode.PLAYBACK) { - return recordedData.removeVariable(); + return getValue(readValue -> readValue, () -> value); + } + + private T getValue(Function readHandler, Supplier valueSupplier) { + if (allowedToReadRecordedValues) { + return readHandler.apply(recordedData.removeVariable()); } else { - recordedData.addVariable(value); + T value = valueSupplier.get(); + + if (allowedToRecordValues) { + recordedData.addVariable(value.toString()); + } + return value; } } diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java new file mode 100644 index 0000000000000..ea092cad62fba --- /dev/null +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.test; + +import org.junit.jupiter.api.Test; + +import java.util.HashMap; + +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for {@link InterceptorManager}. + */ +public class InterceptorManagerTests { + + /** + * Validates that a {@link NullPointerException} is thrown when the test name is null. + */ + @Test + public void nullTestName() { + try { + new InterceptorManager(null, TestMode.RECORD, false); + } catch (Exception ex) { + assertTrue(ex instanceof NullPointerException); + } + + try { + new InterceptorManager(null, new HashMap<>(), false); + } catch (Exception ex) { + assertTrue(ex instanceof NullPointerException); + } + } + + /** + * Validates that {@link InterceptorManager#getRecordedData()} is {@code null} when testing in {@link + * TestMode#LIVE}. + */ + @Test + public void recordedDataIsNullInLiveMode() { + assertNull(new InterceptorManager("testName", TestMode.LIVE, false).getRecordedData()); + assertNull(new InterceptorManager("testName", TestMode.LIVE, true).getRecordedData()); + } + + /** + * Validates that {@link InterceptorManager#getRecordedData()} is {@code null} when {@code doNotRecord} is passed as + * {@code true}. + */ + @Test + public void recordedDataIsNullWhenDoNotRecord() { + assertNull(new InterceptorManager("testName", TestMode.RECORD, true).getRecordedData()); + assertNull(new InterceptorManager("testName", TestMode.LIVE, true).getRecordedData()); + assertNull(new InterceptorManager("testName", TestMode.PLAYBACK, true).getRecordedData()); + assertNull(new InterceptorManager("testName", new HashMap<>(), true).getRecordedData()); + } +} diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java new file mode 100644 index 0000000000000..3c68536fe3e8d --- /dev/null +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.test; + +import com.azure.core.test.annotation.DoNotRecord; +import org.junit.jupiter.api.Test; +import org.opentest4j.TestAbortedException; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for {@link TestContextManager}. + */ +public class TestContextManagerTests { + + /** + * Validates that a test method without the {@link DoNotRecord} annotation is allowed to run in all test modes and + * will record network calls and test values. + */ + @Test + public void testWithoutDoNotRecord() throws NoSuchMethodException { + TestContextManager verifier = new TestContextManager(TestHelper.class.getMethod("testWithoutDoNotRecord")); + + assertFalse(verifier.doNotRecordTest()); + + verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); + assertTrue(verifier.wasTestRan()); + + verifier.verifyTestCanRunInTestMode(TestMode.LIVE); + assertTrue(verifier.wasTestRan()); + + verifier.verifyTestCanRunInTestMode(TestMode.RECORD); + assertTrue(verifier.wasTestRan()); + } + + /** + * Validates that a test method with the default {@link DoNotRecord} annotation is allowed to run in all test modes + * but doesn't have its network calls or test values recorded. + */ + @Test + public void testWithDoNotRecordRunInPlayback() throws NoSuchMethodException { + TestContextManager verifier = new TestContextManager(TestHelper.class.getMethod("testWithDoNotRecordRunInPlayback")); + + assertTrue(verifier.doNotRecordTest()); + + verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); + assertTrue(verifier.wasTestRan()); + + verifier.verifyTestCanRunInTestMode(TestMode.LIVE); + assertTrue(verifier.wasTestRan()); + + verifier.verifyTestCanRunInTestMode(TestMode.RECORD); + assertTrue(verifier.wasTestRan()); + } + + /** + * Validates that a test method with the {@link DoNotRecord} annotation having {@code skipInPlayback} set to true + * is only allowed to run in {@link TestMode#RECORD} and {@link TestMode#LIVE} and won't have its network calls or + * test values recorded. + */ + @Test + public void testWithDoNotRecordSkipInPlayback() throws NoSuchMethodException { + TestContextManager verifier = + new TestContextManager(TestHelper.class.getMethod("testWithDoNotRecordSkipInPlayback")); + + assertTrue(verifier.doNotRecordTest()); + + try { + verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); + } catch (RuntimeException ex) { + assertTrue(ex instanceof TestAbortedException); + } + + verifier.verifyTestCanRunInTestMode(TestMode.LIVE); + assertTrue(verifier.wasTestRan()); + + verifier.verifyTestCanRunInTestMode(TestMode.RECORD); + assertTrue(verifier.wasTestRan()); + } + + static class TestHelper { + public void testWithoutDoNotRecord() { + } + + @DoNotRecord + public void testWithDoNotRecordRunInPlayback() { + + } + + @DoNotRecord(skipInPlayback = true) + public void testWithDoNotRecordSkipInPlayback() { + } + } +} diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java new file mode 100644 index 0000000000000..41aaf57e86b4f --- /dev/null +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.test.utils; + +import com.azure.core.test.TestMode; +import com.azure.core.test.models.RecordedData; +import org.junit.jupiter.api.Test; + +import java.util.NoSuchElementException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests for {@link TestResourceNamer}. + */ +public class TestResourceNamerTests { + private static final String A_VARIABLE = "aVariable"; + private static final String RANDOM_NAME_PREFIX = "prefix"; + private static final int RANDOM_NAME_LENGTH = 12; + private static final String CONFIG_VALUE = "value"; + + /** + * Validates that a {@link NullPointerException} is thrown if {@code testMode} isn't {@link TestMode#LIVE} and + * {@code doNotRecord} is {@code false}, otherwise no exception is thrown as having no {@link RecordedData} is + * valid in those cases. + */ + @Test + public void nullRecordedData() { + // Doesn't throw when TestMode.LIVE or doNotRecord = true + new TestResourceNamer("testName", TestMode.LIVE, false, null); + new TestResourceNamer("testName", TestMode.RECORD, true, null); + + // Does throw when TestMode isn't LIVE and doNotRecord = false + try { + new TestResourceNamer("testName", TestMode.RECORD, false, null); + } catch (Exception ex) { + assertTrue(ex instanceof NullPointerException); + } + + try { + new TestResourceNamer("testName", TestMode.PLAYBACK, false, null); + } catch (Exception ex) { + assertTrue(ex instanceof NullPointerException); + } + } + + /** + * Validates that when {@code doNotRecord} is {@code true} and {@code testMode} is {@link TestMode#PLAYBACK} then + * the {@link RecordedData} within the {@link TestResourceNamer} cannot be read. + */ + @Test + public void recordedDataIsNotAllowedToReadRecordedValues() { + TestResourceNamer resourceNamer = new TestResourceNamer("testName", TestMode.PLAYBACK, true, + getRecordedDataWithValue()); + + assertNotEquals(A_VARIABLE, resourceNamer.randomName("prefix", 12)); + assertNotEquals(A_VARIABLE, resourceNamer.randomUuid()); + assertNotEquals(A_VARIABLE, resourceNamer.now()); + assertEquals(A_VARIABLE, resourceNamer.recordValueFromConfig(A_VARIABLE)); + } + + /** + * Validates that when {@code testMode} is {@link TestMode#LIVE} or {@code doNotRecord} is {@code true} then the + * {@link RecordedData} within the {@link TestResourceNamer} cannot record values generated. + */ + @Test + public void recordedDataIsNotAllowedToRecordValues() { + RecordedData recordedData = new RecordedData(); + + callNamerMethds(new TestResourceNamer("testName", TestMode.LIVE, false, recordedData)); + validateNoRecordingsMade(new TestResourceNamer("testName", TestMode.PLAYBACK, false, recordedData)); + + // Reset the recording data. + recordedData = new RecordedData(); + + callNamerMethds(new TestResourceNamer("testName", TestMode.RECORD, true, recordedData)); + validateNoRecordingsMade(new TestResourceNamer("testName", TestMode.PLAYBACK, false, recordedData)); + } + + private void callNamerMethds(TestResourceNamer resourceNamer) { + resourceNamer.randomName(RANDOM_NAME_PREFIX, RANDOM_NAME_LENGTH); + resourceNamer.randomUuid(); + resourceNamer.now(); + resourceNamer.recordValueFromConfig(CONFIG_VALUE); + } + + private void validateNoRecordingsMade(TestResourceNamer resourceNamer) { + assertNoSuchElementException(() -> resourceNamer.randomName(RANDOM_NAME_PREFIX, RANDOM_NAME_LENGTH)); + assertNoSuchElementException(resourceNamer::randomUuid); + assertNoSuchElementException(resourceNamer::now); + assertNoSuchElementException(() -> resourceNamer.recordValueFromConfig(CONFIG_VALUE)); + } + + private void assertNoSuchElementException(Runnable runnable) { + try { + runnable.run(); + fail("Expected 'NoSuchElementException' to be thrown."); + } catch (Exception ex) { + assertTrue(ex instanceof NoSuchElementException); + } + } + + private RecordedData getRecordedDataWithValue() { + RecordedData recordedData = new RecordedData(); + recordedData.addVariable(A_VARIABLE); + + return recordedData; + } +} From fde7ba41240b20505236a4869ebb8febcfe15eb0 Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Thu, 5 Dec 2019 15:49:02 -0800 Subject: [PATCH 2/6] Fix linting issue --- .../main/java/com/azure/core/test/InterceptorManager.java | 8 ++++---- .../main/java/com/azure/core/test/TestContextManager.java | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 505e8aaabcd23..a369b354e4c05 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -115,8 +115,8 @@ public InterceptorManager(String testName, TestMode testMode, boolean doNotRecor } /** - * Creates a new InterceptorManager that replays test session records. It takes a set of {@code - * textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link + * Creates a new InterceptorManager that replays test session records. It takes a set of + * {@code textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link * NetworkCallRecord#getResponse()}. * * The test session records are read from: "session-records/{@code testName}.json" @@ -135,8 +135,8 @@ public InterceptorManager(String testName, Map textReplacementRu } /** - * Creates a new InterceptorManager that replays test session records. It takes a set of {@code - * textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link + * Creates a new InterceptorManager that replays test session records. It takes a set of + * {@code textReplacementRules}, that can be used by {@link PlaybackClient} to replace values in a {@link * NetworkCallRecord#getResponse()}. * * The test session records are read from: "session-records/{@code testName}.json" diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java index 9b1ab4432e0e9..1f989b3a98c38 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java @@ -28,8 +28,8 @@ public TestContextManager(Method testMethod) { this.doNotRecord = true; this.skipInPlayback = doNotRecordAnnotation.skipInPlayback(); } else { - this.doNotRecord = false; - this.skipInPlayback = false; + this.doNotRecord = false; + this.skipInPlayback = false; } } @@ -44,7 +44,8 @@ public void verifyTestCanRunInTestMode(TestMode testMode) { } /** - * Returns whether the test should have its network calls recorded during a {@link TestMode#RECORD record} test run. + * Returns whether the test should have its network calls recorded during a {@link TestMode#RECORD record} test + * run. * * @return Flag indicating whether to record test network calls. */ From aba6bb2168bc6b0597ddd5b6516e9f569847bce6 Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Fri, 6 Dec 2019 15:16:56 -0800 Subject: [PATCH 3/6] Updated new constructors to take TestContextManager instead of splayed parameters --- .../azure/core/test/InterceptorManager.java | 14 +++++--- .../java/com/azure/core/test/TestBase.java | 19 ++++------- .../azure/core/test/TestContextManager.java | 32 ++++++++++++++----- .../core/test/utils/TestResourceNamer.java | 15 ++++++--- .../core/test/TestContextManagerTests.java | 16 +++++----- 5 files changed, 58 insertions(+), 38 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index a369b354e4c05..2c411e68b96e7 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -68,7 +68,7 @@ public class InterceptorManager implements AutoCloseable { * @throws UncheckedIOException If {@code testMode} is {@link TestMode#PLAYBACK} and an existing test session record * could not be located or the data could not be deserialized into an instance of {@link RecordedData}. * @throws NullPointerException If {@code testName} is {@code null}. - * @deprecated Use {@link #InterceptorManager(String, TestMode, boolean)} instead. + * @deprecated Use {@link #InterceptorManager(TestContextManager)} instead. */ @Deprecated public InterceptorManager(String testName, TestMode testMode) { @@ -89,19 +89,23 @@ public InterceptorManager(String testName, TestMode testMode) { * * The test session records are persisted in the path: "session-records/{@code testName}.json" * - * @param testName Name of the test session record. - * @param testMode The {@link TestMode} for this interceptor. - * @param doNotRecord Flag indicating whether network calls should be record or played back. + * @param testContextManager Contextual information about the test being ran, such as test name, {@link TestMode}, + * and others. * @throws UncheckedIOException If {@code testMode} is {@link TestMode#PLAYBACK} and an existing test session record * could not be located or the data could not be deserialized into an instance of {@link RecordedData}. * @throws NullPointerException If {@code testName} is {@code null}. */ - public InterceptorManager(String testName, TestMode testMode, boolean doNotRecord) { + public InterceptorManager(TestContextManager testContextManager) { + this(testContextManager.getTestName(), testContextManager.getTestMode(), testContextManager.doNotRecordTest()); + } + + private InterceptorManager(String testName, TestMode testMode, boolean doNotRecord) { Objects.requireNonNull(testName, "'testName' cannot be null."); this.testName = testName; this.testMode = testMode; this.textReplacementRules = new HashMap<>(); + this.allowedToReadRecordedValues = (testMode == TestMode.PLAYBACK && !doNotRecord); this.allowedToRecordValues = (testMode == TestMode.RECORD && !doNotRecord); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java index ce2c2fc055cae..a1294ddebbe42 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestBase.java @@ -23,7 +23,7 @@ public abstract class TestBase implements BeforeEachCallback { // Environment variable name used to determine the TestMode. private static final String AZURE_TEST_MODE = "AZURE_TEST_MODE"; - static TestMode testMode; + private static TestMode testMode; private final ClientLogger logger = new ClientLogger(TestBase.class); @@ -55,21 +55,16 @@ public void beforeEach(ExtensionContext extensionContext) { */ @BeforeEach public void setupTest(TestInfo testInfo) { - final Method testMethod = testInfo.getTestMethod().get(); - this.testContextManager = new TestContextManager(testMethod); - testContextManager.verifyTestCanRunInTestMode(testMode); - - final String testName = testMethod.getName(); - logger.info("Test Mode: {}, Name: {}", testMode, testName); + this.testContextManager = new TestContextManager(testInfo.getTestMethod().get(), testMode); + logger.info("Test Mode: {}, Name: {}", testMode, testContextManager.getTestName()); try { - interceptorManager = new InterceptorManager(testName, testMode, testContextManager.doNotRecordTest()); + interceptorManager = new InterceptorManager(testContextManager); } catch (UncheckedIOException e) { - logger.error("Could not create interceptor for {}", testName, e); + logger.error("Could not create interceptor for {}", testContextManager.getTestName(), e); Assertions.fail(); } - testResourceNamer = new TestResourceNamer(testName, testMode, testContextManager.doNotRecordTest(), - interceptorManager.getRecordedData()); + testResourceNamer = new TestResourceNamer(testContextManager, interceptorManager.getRecordedData()); beforeTest(); } @@ -80,7 +75,7 @@ public void setupTest(TestInfo testInfo) { */ @AfterEach public void teardownTest(TestInfo testInfo) { - if (testContextManager.wasTestRan()) { + if (testContextManager.didTestRun()) { afterTest(); interceptorManager.close(); } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java index 1f989b3a98c38..75695fc7776d9 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java @@ -13,16 +13,21 @@ * is capable of running. */ public class TestContextManager { + private final String testName; + private final TestMode testMode; private final boolean doNotRecord; private final boolean skipInPlayback; - private volatile boolean testRan; + private final boolean testRan; /** * Constructs a {@link TestContextManager} based on the test method. * * @param testMethod Test method being ran. */ - public TestContextManager(Method testMethod) { + public TestContextManager(Method testMethod, TestMode testMode) { + this.testName = testMethod.getName(); + this.testMode = testMode; + DoNotRecord doNotRecordAnnotation = testMethod.getAnnotation(DoNotRecord.class); if (doNotRecordAnnotation != null) { this.doNotRecord = true; @@ -31,16 +36,27 @@ public TestContextManager(Method testMethod) { this.doNotRecord = false; this.skipInPlayback = false; } + + this.testRan = !(skipInPlayback && testMode == TestMode.PLAYBACK); + assumeTrue(testRan, "Test does not allow playback and was ran in 'TestMode.PLAYBACK'"); } /** - * Verifies whether the current test is allowed to run. + * Returns the name of the test being ran. * - * @param testMode The {@link TestMode} tests are being ran in. + * @return The test name. */ - public void verifyTestCanRunInTestMode(TestMode testMode) { - this.testRan = !(skipInPlayback && testMode == TestMode.PLAYBACK); - assumeTrue(testRan, "Test ddes not allow playback and was ran in 'TestMode.PLAYBACK'"); + public String getTestName() { + return testName; + } + + /** + * Returns the mode being used to run tests. + * + * @return The {@link TestMode} being used to run tests. + */ + public TestMode getTestMode() { + return testMode; } /** @@ -58,7 +74,7 @@ public boolean doNotRecordTest() { * * @return Flag indicating whether the current test was ran. */ - public boolean wasTestRan() { + public boolean didTestRun() { return testRan; } } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java index 31ca4488fd2fc..3b38a1d2e0852 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestResourceNamer.java @@ -3,6 +3,7 @@ package com.azure.core.test.utils; +import com.azure.core.test.TestContextManager; import com.azure.core.test.TestMode; import com.azure.core.test.models.RecordedData; @@ -25,7 +26,7 @@ public class TestResourceNamer extends ResourceNamer { /** * Constructor of TestResourceNamer * - * @deprecated Use {@link #TestResourceNamer(String, TestMode, boolean, RecordedData)} instead. + * @deprecated Use {@link #TestResourceNamer(TestContextManager, RecordedData)} instead. * @param name test name as prefix * @param testMode The {@link TestMode} which the test is running in. * @param recordedData the recorded data with list of network call @@ -38,14 +39,18 @@ public TestResourceNamer(String name, TestMode testMode, RecordedData recordedDa /** * Constructor of TestResourceNamer * - * @param name test name as prefix - * @param testMode The {@link TestMode} which the test is running in. - * @param doNotRecord Flag indicating whether values produced by this class should be recorded or played back. + * @param testContextManager Contextual information about the test being ran, such as test name, {@link TestMode}, + * and others. * @param recordedData the recorded data with list of network call * @throws NullPointerException If {@code testMode} isn't {@link TestMode#LIVE}, {@code doNotRecord} is * {@code false}, and {@code recordedData} is {@code null}. */ - public TestResourceNamer(String name, TestMode testMode, boolean doNotRecord, RecordedData recordedData) { + public TestResourceNamer(TestContextManager testContextManager, RecordedData recordedData) { + this(testContextManager.getTestName(), testContextManager.getTestMode(), testContextManager.doNotRecordTest(), + recordedData); + } + + private TestResourceNamer(String name, TestMode testMode, boolean doNotRecord, RecordedData recordedData) { super(name); // Only need recordedData if the test is running in playback or record. diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java index 3c68536fe3e8d..e111158220aff 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java @@ -26,13 +26,13 @@ public void testWithoutDoNotRecord() throws NoSuchMethodException { assertFalse(verifier.doNotRecordTest()); verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); } /** @@ -46,13 +46,13 @@ public void testWithDoNotRecordRunInPlayback() throws NoSuchMethodException { assertTrue(verifier.doNotRecordTest()); verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); } /** @@ -74,10 +74,10 @@ public void testWithDoNotRecordSkipInPlayback() throws NoSuchMethodException { } verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.wasTestRan()); + assertTrue(verifier.didTestRun()); } static class TestHelper { From 28b5d289d5f8562a9970492c145bf3b8e5894a82 Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Dec 2019 09:54:11 -0800 Subject: [PATCH 4/6] Updated unit tests based on code changes --- sdk/core/azure-core-test/pom.xml | 6 ++ .../com/azure/core/test/FakeTestClass.java | 35 ++++++++ .../core/test/InterceptorManagerTests.java | 37 +++----- .../core/test/TestContextManagerTests.java | 89 +++++++------------ .../test/utils/TestResourceNamerTests.java | 58 ++++++------ 5 files changed, 114 insertions(+), 111 deletions(-) create mode 100644 sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java diff --git a/sdk/core/azure-core-test/pom.xml b/sdk/core/azure-core-test/pom.xml index 7b8789f9786e8..718b54378f3d8 100644 --- a/sdk/core/azure-core-test/pom.xml +++ b/sdk/core/azure-core-test/pom.xml @@ -84,6 +84,12 @@ jetty-server 9.4.11.v20180605 + + org.junit.jupiter + junit-jupiter-params + 5.4.2 + test + diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java new file mode 100644 index 0000000000000..2c8d354849bfe --- /dev/null +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java @@ -0,0 +1,35 @@ +package com.azure.core.test; + +import com.azure.core.test.annotation.DoNotRecord; +import com.azure.core.util.logging.ClientLogger; + +import java.lang.reflect.Method; + +/** + * This is a dummy class used to mock scenarios for {@link TestContextManager}. + */ +public final class FakeTestClass { + public static final Method METHOD_WITHOUT_DONOTRECORD = getTestMethod("testWithoutDoNotRecord"); + public static final Method DONOTRECORD_FALSE_SKIPINPLAYBACK = getTestMethod("testWithDoNotRecordRunInPlayback"); + public static final Method DONOTRECORD_SKIPINPLAYBACK = getTestMethod("testWithDoNotRecordSkipInPlayback"); + + public void testWithoutDoNotRecord() { + } + + @DoNotRecord + public void testWithDoNotRecordRunInPlayback() { + + } + + @DoNotRecord(skipInPlayback = true) + public void testWithDoNotRecordSkipInPlayback() { + } + + private static Method getTestMethod(String methodName) { + try { + return FakeTestClass.class.getMethod(methodName); + } catch (NoSuchMethodException e) { + throw new ClientLogger(FakeTestClass.class).logExceptionAsWarning(new RuntimeException(e)); + } + } +} diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java index ea092cad62fba..4d953e8a278eb 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/InterceptorManagerTests.java @@ -7,40 +7,24 @@ import java.util.HashMap; +import static com.azure.core.test.FakeTestClass.DONOTRECORD_FALSE_SKIPINPLAYBACK; +import static com.azure.core.test.FakeTestClass.METHOD_WITHOUT_DONOTRECORD; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * Tests for {@link InterceptorManager}. */ public class InterceptorManagerTests { - - /** - * Validates that a {@link NullPointerException} is thrown when the test name is null. - */ - @Test - public void nullTestName() { - try { - new InterceptorManager(null, TestMode.RECORD, false); - } catch (Exception ex) { - assertTrue(ex instanceof NullPointerException); - } - - try { - new InterceptorManager(null, new HashMap<>(), false); - } catch (Exception ex) { - assertTrue(ex instanceof NullPointerException); - } - } - /** * Validates that {@link InterceptorManager#getRecordedData()} is {@code null} when testing in {@link * TestMode#LIVE}. */ @Test public void recordedDataIsNullInLiveMode() { - assertNull(new InterceptorManager("testName", TestMode.LIVE, false).getRecordedData()); - assertNull(new InterceptorManager("testName", TestMode.LIVE, true).getRecordedData()); + assertNull(new InterceptorManager(new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.LIVE)) + .getRecordedData()); + assertNull(new InterceptorManager(new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.LIVE)) + .getRecordedData()); } /** @@ -49,9 +33,12 @@ public void recordedDataIsNullInLiveMode() { */ @Test public void recordedDataIsNullWhenDoNotRecord() { - assertNull(new InterceptorManager("testName", TestMode.RECORD, true).getRecordedData()); - assertNull(new InterceptorManager("testName", TestMode.LIVE, true).getRecordedData()); - assertNull(new InterceptorManager("testName", TestMode.PLAYBACK, true).getRecordedData()); + assertNull(new InterceptorManager(new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.RECORD)) + .getRecordedData()); + assertNull(new InterceptorManager(new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.LIVE)) + .getRecordedData()); + assertNull(new InterceptorManager(new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.PLAYBACK)) + .getRecordedData()); assertNull(new InterceptorManager("testName", new HashMap<>(), true).getRecordedData()); } } diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java index e111158220aff..b36654d276b31 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestContextManagerTests.java @@ -5,9 +5,17 @@ import com.azure.core.test.annotation.DoNotRecord; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.opentest4j.TestAbortedException; +import java.lang.reflect.Method; + +import static com.azure.core.test.FakeTestClass.DONOTRECORD_FALSE_SKIPINPLAYBACK; +import static com.azure.core.test.FakeTestClass.DONOTRECORD_SKIPINPLAYBACK; +import static com.azure.core.test.FakeTestClass.METHOD_WITHOUT_DONOTRECORD; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -19,78 +27,45 @@ public class TestContextManagerTests { * Validates that a test method without the {@link DoNotRecord} annotation is allowed to run in all test modes and * will record network calls and test values. */ - @Test - public void testWithoutDoNotRecord() throws NoSuchMethodException { - TestContextManager verifier = new TestContextManager(TestHelper.class.getMethod("testWithoutDoNotRecord")); - - assertFalse(verifier.doNotRecordTest()); - - verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); - assertTrue(verifier.didTestRun()); + @ParameterizedTest(name = "[{index}] {displayName}") + @EnumSource(TestMode.class) + public void testWithoutDoNotRecord(TestMode testMode) { + TestContextManager testContextManager = new TestContextManager(METHOD_WITHOUT_DONOTRECORD, testMode); - verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.didTestRun()); - - verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.didTestRun()); + assertFalse(testContextManager.doNotRecordTest()); + assertTrue(testContextManager.didTestRun()); } /** * Validates that a test method with the default {@link DoNotRecord} annotation is allowed to run in all test modes * but doesn't have its network calls or test values recorded. */ - @Test - public void testWithDoNotRecordRunInPlayback() throws NoSuchMethodException { - TestContextManager verifier = new TestContextManager(TestHelper.class.getMethod("testWithDoNotRecordRunInPlayback")); - - assertTrue(verifier.doNotRecordTest()); - - verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); - assertTrue(verifier.didTestRun()); + @ParameterizedTest(name = "[{index}] {displayName}") + @EnumSource(TestMode.class) + public void testWithDoNotRecordRunInPlayback(TestMode testMode) { + TestContextManager testContextManager = new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, testMode); - verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.didTestRun()); - - verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.didTestRun()); + assertTrue(testContextManager.doNotRecordTest()); + assertTrue(testContextManager.didTestRun()); } /** - * Validates that a test method with the {@link DoNotRecord} annotation having {@code skipInPlayback} set to true - * is only allowed to run in {@link TestMode#RECORD} and {@link TestMode#LIVE} and won't have its network calls or - * test values recorded. + * Validates that a test method with the {@link DoNotRecord} annotation having {@code skipInPlayback} set to true is + * only allowed to run in {@link TestMode#RECORD} and {@link TestMode#LIVE} and won't have its network calls or test + * values recorded. */ @Test - public void testWithDoNotRecordSkipInPlayback() throws NoSuchMethodException { - TestContextManager verifier = - new TestContextManager(TestHelper.class.getMethod("testWithDoNotRecordSkipInPlayback")); - - assertTrue(verifier.doNotRecordTest()); - - try { - verifier.verifyTestCanRunInTestMode(TestMode.PLAYBACK); - } catch (RuntimeException ex) { - assertTrue(ex instanceof TestAbortedException); - } - - verifier.verifyTestCanRunInTestMode(TestMode.LIVE); - assertTrue(verifier.didTestRun()); - - verifier.verifyTestCanRunInTestMode(TestMode.RECORD); - assertTrue(verifier.didTestRun()); - } - - static class TestHelper { - public void testWithoutDoNotRecord() { - } + public void testWithDoNotRecordSkipInPlayback() { + Method testMethod = DONOTRECORD_SKIPINPLAYBACK; - @DoNotRecord - public void testWithDoNotRecordRunInPlayback() { + assertThrows(TestAbortedException.class, () -> new TestContextManager(testMethod, TestMode.PLAYBACK)); - } + TestContextManager testContextManager = new TestContextManager(testMethod, TestMode.LIVE); + assertTrue(testContextManager.doNotRecordTest()); + assertTrue(testContextManager.didTestRun()); - @DoNotRecord(skipInPlayback = true) - public void testWithDoNotRecordSkipInPlayback() { - } + testContextManager = new TestContextManager(testMethod, TestMode.RECORD); + assertTrue(testContextManager.doNotRecordTest()); + assertTrue(testContextManager.didTestRun()); } } diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java index 41aaf57e86b4f..94046262148fe 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/utils/TestResourceNamerTests.java @@ -3,16 +3,20 @@ package com.azure.core.test.utils; +import com.azure.core.test.TestContextManager; import com.azure.core.test.TestMode; import com.azure.core.test.models.RecordedData; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import java.util.NoSuchElementException; +import static com.azure.core.test.FakeTestClass.DONOTRECORD_FALSE_SKIPINPLAYBACK; +import static com.azure.core.test.FakeTestClass.METHOD_WITHOUT_DONOTRECORD; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * Tests for {@link TestResourceNamer}. @@ -30,22 +34,19 @@ public class TestResourceNamerTests { */ @Test public void nullRecordedData() { - // Doesn't throw when TestMode.LIVE or doNotRecord = true - new TestResourceNamer("testName", TestMode.LIVE, false, null); - new TestResourceNamer("testName", TestMode.RECORD, true, null); + // Doesn't throw when TestMode.LIVE. + assertDoesNotThrow(() -> + new TestResourceNamer(new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.LIVE), null)); + + // Doesn't throw when 'doNotRecord' is true. + assertDoesNotThrow(() -> + new TestResourceNamer(new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.RECORD), null)); // Does throw when TestMode isn't LIVE and doNotRecord = false - try { - new TestResourceNamer("testName", TestMode.RECORD, false, null); - } catch (Exception ex) { - assertTrue(ex instanceof NullPointerException); - } - - try { - new TestResourceNamer("testName", TestMode.PLAYBACK, false, null); - } catch (Exception ex) { - assertTrue(ex instanceof NullPointerException); - } + assertThrows(NullPointerException.class, () -> + new TestResourceNamer(new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.RECORD), null)); + assertThrows(NullPointerException.class, () -> + new TestResourceNamer(new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.PLAYBACK), null)); } /** @@ -54,8 +55,8 @@ public void nullRecordedData() { */ @Test public void recordedDataIsNotAllowedToReadRecordedValues() { - TestResourceNamer resourceNamer = new TestResourceNamer("testName", TestMode.PLAYBACK, true, - getRecordedDataWithValue()); + TestResourceNamer resourceNamer = new TestResourceNamer(new TestContextManager( + DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.PLAYBACK), getRecordedDataWithValue()); assertNotEquals(A_VARIABLE, resourceNamer.randomName("prefix", 12)); assertNotEquals(A_VARIABLE, resourceNamer.randomUuid()); @@ -71,14 +72,18 @@ public void recordedDataIsNotAllowedToReadRecordedValues() { public void recordedDataIsNotAllowedToRecordValues() { RecordedData recordedData = new RecordedData(); - callNamerMethds(new TestResourceNamer("testName", TestMode.LIVE, false, recordedData)); - validateNoRecordingsMade(new TestResourceNamer("testName", TestMode.PLAYBACK, false, recordedData)); + callNamerMethds(new TestResourceNamer( + new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.LIVE), recordedData)); + validateNoRecordingsMade(new TestResourceNamer( + new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.PLAYBACK), recordedData)); // Reset the recording data. recordedData = new RecordedData(); - callNamerMethds(new TestResourceNamer("testName", TestMode.RECORD, true, recordedData)); - validateNoRecordingsMade(new TestResourceNamer("testName", TestMode.PLAYBACK, false, recordedData)); + callNamerMethds(new TestResourceNamer( + new TestContextManager(DONOTRECORD_FALSE_SKIPINPLAYBACK, TestMode.RECORD), recordedData)); + validateNoRecordingsMade(new TestResourceNamer( + new TestContextManager(METHOD_WITHOUT_DONOTRECORD, TestMode.PLAYBACK), recordedData)); } private void callNamerMethds(TestResourceNamer resourceNamer) { @@ -95,13 +100,8 @@ private void validateNoRecordingsMade(TestResourceNamer resourceNamer) { assertNoSuchElementException(() -> resourceNamer.recordValueFromConfig(CONFIG_VALUE)); } - private void assertNoSuchElementException(Runnable runnable) { - try { - runnable.run(); - fail("Expected 'NoSuchElementException' to be thrown."); - } catch (Exception ex) { - assertTrue(ex instanceof NoSuchElementException); - } + private void assertNoSuchElementException(Executable executable) { + assertThrows(NoSuchElementException.class, executable, "Expected 'NoSuchElementException' to be thrown."); } private RecordedData getRecordedDataWithValue() { From 25b66ff542e4399c24f7fbcdfa9cf4da4e409abf Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Dec 2019 11:12:45 -0800 Subject: [PATCH 5/6] Add missing dependency comment --- sdk/core/azure-core-test/pom.xml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/core/azure-core-test/pom.xml b/sdk/core/azure-core-test/pom.xml index 90127cab7e1a9..45501824e3708 100644 --- a/sdk/core/azure-core-test/pom.xml +++ b/sdk/core/azure-core-test/pom.xml @@ -59,6 +59,12 @@ 5.4.2 test + + org.junit.jupiter + junit-jupiter-params + 5.4.2 + test + org.slf4j slf4j-api @@ -84,12 +90,6 @@ jetty-server 9.4.11.v20180605 - - org.junit.jupiter - junit-jupiter-params - 5.4.2 - test - From 4e9c322138e1eac3fb30f8555a801810a231aa79 Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Dec 2019 11:32:01 -0800 Subject: [PATCH 6/6] Fixing linting issues --- .../src/main/java/com/azure/core/test/TestContextManager.java | 1 + .../src/test/java/com/azure/core/test/FakeTestClass.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java index 75695fc7776d9..6e7eb696b77c9 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/TestContextManager.java @@ -23,6 +23,7 @@ public class TestContextManager { * Constructs a {@link TestContextManager} based on the test method. * * @param testMethod Test method being ran. + * @param testMode The {@link TestMode} the test is running in. */ public TestContextManager(Method testMethod, TestMode testMode) { this.testName = testMethod.getName(); diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java index 2c8d354849bfe..7dfe22900ffb5 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/FakeTestClass.java @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. package com.azure.core.test; import com.azure.core.test.annotation.DoNotRecord;