diff --git a/java/google/registry/config/RegistryEnvironment.java b/java/google/registry/config/RegistryEnvironment.java index 1c6c373ea..cc3b2a558 100644 --- a/java/google/registry/config/RegistryEnvironment.java +++ b/java/google/registry/config/RegistryEnvironment.java @@ -47,11 +47,25 @@ public enum RegistryEnvironment { */ UNITTEST; + /** Sets this enum as the name of the registry environment. */ + public RegistryEnvironment setup() { + return setup(SystemPropertySetter.PRODUCTION_IMPL); + } + + /** + * Sets this enum as the name of the registry environment using specified {@link + * SystemPropertySetter}. + */ + public RegistryEnvironment setup(SystemPropertySetter systemPropertySetter) { + systemPropertySetter.setProperty(PROPERTY, name()); + return this; + } + /** Returns environment configured by system property {@value #PROPERTY}. */ public static RegistryEnvironment get() { return valueOf(Ascii.toUpperCase(System.getProperty(PROPERTY, UNITTEST.name()))); } /** System property for configuring which environment we should use. */ - public static final String PROPERTY = "google.registry.environment"; + private static final String PROPERTY = "google.registry.environment"; } diff --git a/java/google/registry/config/SystemPropertySetter.java b/java/google/registry/config/SystemPropertySetter.java new file mode 100644 index 000000000..41cd864ea --- /dev/null +++ b/java/google/registry/config/SystemPropertySetter.java @@ -0,0 +1,47 @@ +// Copyright 2018 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.config; + +import javax.annotation.Nullable; + +/** + * Wrapper interface around {@link System#setProperty(String, String)} and {@link + * System#clearProperty(String)}. Tests that modify system properties may provide custom + * implementations that keeps track of changes and restores original property values on test + * completion. + */ +public interface SystemPropertySetter { + + /** + * Updates the system property specified by {@code key}. If {@code value} is not null, {@link + * System#setProperty(String, String)} is invoked; otherwise {@link System#clearProperty(String)} + * is invoked. + */ + SystemPropertySetter setProperty(String key, @Nullable String value); + + /** Production implementation of {@link SystemPropertySetter}. */ + SystemPropertySetter PRODUCTION_IMPL = + new SystemPropertySetter() { + @Override + public SystemPropertySetter setProperty(String key, @Nullable String value) { + if (value == null) { + System.clearProperty(key); + } else { + System.setProperty(key, value); + } + return this; + } + }; +} diff --git a/java/google/registry/tools/RegistryToolEnvironment.java b/java/google/registry/tools/RegistryToolEnvironment.java index 26006167f..f2dff040d 100644 --- a/java/google/registry/tools/RegistryToolEnvironment.java +++ b/java/google/registry/tools/RegistryToolEnvironment.java @@ -17,10 +17,12 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.config.RegistryEnvironment; +import google.registry.config.SystemPropertySetter; /** Enum of production environments, used for the {@code --environment} flag. */ enum RegistryToolEnvironment { @@ -75,12 +77,24 @@ enum RegistryToolEnvironment { return instance; } - /** Setup execution environment. Call this method before any classes are loaded. */ + /** Resets static class state to uninitialized state. */ + @VisibleForTesting + static void reset() { + instance = null; + } + + /** Sets up execution environment. Call this method before any classes are loaded. */ void setup() { + setup(SystemPropertySetter.PRODUCTION_IMPL); + } + + /** Sets up execution environment. Call this method before any classes are loaded. */ + @VisibleForTesting + void setup(SystemPropertySetter systemPropertySetter) { instance = this; - System.setProperty(RegistryEnvironment.PROPERTY, actualEnvironment.name()); + actualEnvironment.setup(systemPropertySetter); for (ImmutableMap.Entry entry : extraProperties.entrySet()) { - System.setProperty(entry.getKey(), entry.getValue()); + systemPropertySetter.setProperty(entry.getKey(), entry.getValue()); } } diff --git a/javatests/google/registry/config/BUILD b/javatests/google/registry/config/BUILD index f9c08acbf..673cd3adf 100644 --- a/javatests/google/registry/config/BUILD +++ b/javatests/google/registry/config/BUILD @@ -12,6 +12,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//java/google/registry/config", + "//javatests/google/registry/testing", "@com_google_auto_value", "@com_google_guava", "@com_google_truth", diff --git a/javatests/google/registry/testing/SystemPropertyRule.java b/javatests/google/registry/testing/SystemPropertyRule.java index 19cee6302..71fb09392 100644 --- a/javatests/google/registry/testing/SystemPropertyRule.java +++ b/javatests/google/registry/testing/SystemPropertyRule.java @@ -17,6 +17,7 @@ package google.registry.testing; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import google.registry.config.SystemPropertySetter; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -25,10 +26,8 @@ import java.util.Optional; import javax.annotation.Nullable; import org.junit.rules.ExternalResource; -/** - * JUnit Rule for overriding the values Java system properties during tests. - */ -public final class SystemPropertyRule extends ExternalResource { +/** JUnit Rule for overriding the values Java system properties during tests. */ +public final class SystemPropertyRule extends ExternalResource implements SystemPropertySetter { /** Class representing a system property key value pair. */ private static class Property { @@ -56,14 +55,15 @@ public final class SystemPropertyRule extends ExternalResource { /** * Change the value of a system property which is restored to its original value after the test. * - *

It's safe to call this method multiple times with the same {@code key} within a single - * test. Only the truly original property value will be restored at the end. + *

It's safe to call this method multiple times with the same {@code key} within a single test. + * Only the truly original property value will be restored at the end. * *

This method can be called fluently when declaring the Rule field, or within a Test method. * * @see java.lang.System#setProperty(String, String) */ - public SystemPropertyRule override(String key, @Nullable String value) { + @Override + public SystemPropertyRule setProperty(String key, @Nullable String value) { originals.computeIfAbsent( key, k -> new Property(k, Optional.ofNullable(System.getProperty(k)))); Property property = new Property(key, Optional.ofNullable(value)); diff --git a/javatests/google/registry/tools/CommandTestCase.java b/javatests/google/registry/tools/CommandTestCase.java index f9b976e96..80ca4fcaa 100644 --- a/javatests/google/registry/tools/CommandTestCase.java +++ b/javatests/google/registry/tools/CommandTestCase.java @@ -32,6 +32,7 @@ import google.registry.model.poll.PollMessage; import google.registry.testing.AppEngineRule; import google.registry.testing.CertificateSamples; import google.registry.testing.MockitoJUnitRule; +import google.registry.testing.SystemPropertyRule; import google.registry.tools.params.ParameterFactory; import java.io.ByteArrayOutputStream; import java.io.File; @@ -61,6 +62,8 @@ public abstract class CommandTestCase { public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().withTaskQueue().build(); + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + @Rule public final MockitoJUnitRule mocks = MockitoJUnitRule.create(); @Rule @@ -69,14 +72,14 @@ public abstract class CommandTestCase { @Before public final void beforeCommandTestCase() { // Ensure the UNITTEST environment has been set before constructing a new command instance. - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); command = newCommandInstance(); System.setOut(new PrintStream(stdout)); System.setErr(new PrintStream(stderr)); } void runCommandInEnvironment(RegistryToolEnvironment env, String... args) throws Exception { - env.setup(); + env.setup(systemPropertyRule); try { JCommander jcommander = new JCommander(command); jcommander.addConverterFactory(new ParameterFactory()); @@ -87,7 +90,7 @@ public abstract class CommandTestCase { // This primarily matters for AutoTimestamp fields, which otherwise won't have updated values. ofy().clearSessionCache(); // Reset back to UNITTEST environment. - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); } } diff --git a/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java b/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java index e6ee2eff5..45978e36a 100644 --- a/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java +++ b/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java @@ -21,7 +21,9 @@ import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpRequestInitializer; import google.registry.config.RegistryConfig; +import google.registry.testing.SystemPropertyRule; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -40,11 +42,13 @@ public class DefaultRequestFactoryModuleTest { } }); + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + DefaultRequestFactoryModule module = new DefaultRequestFactoryModule(); @Before public void setUp() { - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); } @Test diff --git a/javatests/google/registry/tools/GtechToolTest.java b/javatests/google/registry/tools/GtechToolTest.java index 90683073f..a9ddbbad7 100644 --- a/javatests/google/registry/tools/GtechToolTest.java +++ b/javatests/google/registry/tools/GtechToolTest.java @@ -17,7 +17,9 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.Sets; +import google.registry.testing.SystemPropertyRule; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -26,9 +28,11 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class GtechToolTest { + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + @Before public void init() { - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); } @Test diff --git a/javatests/google/registry/tools/RegistryToolEnvironmentTest.java b/javatests/google/registry/tools/RegistryToolEnvironmentTest.java index 6fff5db32..f57ac3cb6 100644 --- a/javatests/google/registry/tools/RegistryToolEnvironmentTest.java +++ b/javatests/google/registry/tools/RegistryToolEnvironmentTest.java @@ -17,6 +17,8 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.JUnitBackports.assertThrows; +import google.registry.testing.SystemPropertyRule; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -25,17 +27,20 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class RegistryToolEnvironmentTest { + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + @Test public void testGet_withoutSetup_throws() { + RegistryToolEnvironment.reset(); assertThrows(IllegalStateException.class, RegistryToolEnvironment::get); } @Test public void testSetup_changesEnvironmentReturnedByGet() { - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); assertThat(RegistryToolEnvironment.get()).isEqualTo(RegistryToolEnvironment.UNITTEST); - RegistryToolEnvironment.ALPHA.setup(); + RegistryToolEnvironment.ALPHA.setup(systemPropertyRule); assertThat(RegistryToolEnvironment.get()).isEqualTo(RegistryToolEnvironment.ALPHA); } diff --git a/javatests/google/registry/tools/RegistryToolTest.java b/javatests/google/registry/tools/RegistryToolTest.java index f7451f596..31171e9d4 100644 --- a/javatests/google/registry/tools/RegistryToolTest.java +++ b/javatests/google/registry/tools/RegistryToolTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.Sets; import com.google.common.reflect.ClassPath; import com.google.common.reflect.ClassPath.ClassInfo; import com.google.common.truth.Expect; +import google.registry.testing.SystemPropertyRule; import java.io.IOException; import java.lang.reflect.Modifier; import java.util.Map; @@ -41,9 +42,11 @@ public class RegistryToolTest { @Rule public final Expect expect = Expect.create(); + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + @Before public void init() { - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); } @Test diff --git a/javatests/google/registry/tools/ShellCommandTest.java b/javatests/google/registry/tools/ShellCommandTest.java index e99ec6d2b..5ae60ccb5 100644 --- a/javatests/google/registry/tools/ShellCommandTest.java +++ b/javatests/google/registry/tools/ShellCommandTest.java @@ -27,6 +27,7 @@ import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.testing.FakeClock; +import google.registry.testing.SystemPropertyRule; import google.registry.tools.ShellCommand.JCommanderCompletor; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -40,6 +41,7 @@ import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -47,6 +49,8 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class ShellCommandTest { + @Rule public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule(); + CommandRunner cli = mock(CommandRunner.class); FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ")); @@ -107,7 +111,7 @@ public class ShellCommandTest { @Test public void testNoIdleWhenInAlpha() throws Exception { - RegistryToolEnvironment.ALPHA.setup(); + RegistryToolEnvironment.ALPHA.setup(systemPropertyRule); MockCli cli = new MockCli(); ShellCommand shellCommand = createShellCommand(cli, Duration.standardDays(1), "test1 foo bar", "test2 foo bar"); @@ -116,7 +120,7 @@ public class ShellCommandTest { @Test public void testNoIdleWhenInSandbox() throws Exception { - RegistryToolEnvironment.SANDBOX.setup(); + RegistryToolEnvironment.SANDBOX.setup(systemPropertyRule); MockCli cli = new MockCli(); ShellCommand shellCommand = createShellCommand(cli, Duration.standardDays(1), "test1 foo bar", "test2 foo bar"); @@ -125,7 +129,7 @@ public class ShellCommandTest { @Test public void testIdleWhenOverHourInProduction() throws Exception { - RegistryToolEnvironment.PRODUCTION.setup(); + RegistryToolEnvironment.PRODUCTION.setup(systemPropertyRule); MockCli cli = new MockCli(); ShellCommand shellCommand = createShellCommand(cli, Duration.standardMinutes(61), "test1 foo bar", "test2 foo bar"); @@ -135,7 +139,7 @@ public class ShellCommandTest { @Test public void testNoIdleWhenUnderHourInProduction() throws Exception { - RegistryToolEnvironment.PRODUCTION.setup(); + RegistryToolEnvironment.PRODUCTION.setup(systemPropertyRule); MockCli cli = new MockCli(); ShellCommand shellCommand = createShellCommand(cli, Duration.standardMinutes(59), "test1 foo bar", "test2 foo bar"); @@ -155,7 +159,7 @@ public class ShellCommandTest { public void testMultipleCommandInvocations() throws Exception { try (RegistryCli cli = new RegistryCli("unittest", ImmutableMap.of("test_command", TestCommand.class))) { - RegistryToolEnvironment.UNITTEST.setup(); + RegistryToolEnvironment.UNITTEST.setup(systemPropertyRule); cli.setEnvironment(RegistryToolEnvironment.UNITTEST); cli.run(new String[] {"test_command", "-x", "xval", "arg1", "arg2"}); cli.run(new String[] {"test_command", "-x", "otherxval", "arg3"}); @@ -272,7 +276,7 @@ public class ShellCommandTest { @Test public void testEncapsulatedOutput_command() throws Exception { - RegistryToolEnvironment.ALPHA.setup(); + RegistryToolEnvironment.ALPHA.setup(systemPropertyRule); captureOutput(); ShellCommand shellCommand = new ShellCommand( @@ -296,7 +300,7 @@ public class ShellCommandTest { @Test public void testEncapsulatedOutput_throws() throws Exception { - RegistryToolEnvironment.ALPHA.setup(); + RegistryToolEnvironment.ALPHA.setup(systemPropertyRule); captureOutput(); ShellCommand shellCommand = new ShellCommand(