Stop provisioning RegistryEnvironment from Dagger (#140)

We found that some webdriver tests failed because RegistryEnvironment
was set to 'production' by other test and was carried over to the
webdriver test, and it was nontrivial to fix them because the instance
of RegistryEnvironment was injected into the testing web server.

Also, to eliminate this problem from potential victims, we stopped
provisioning RegistryEnvironment from Dagger. Instead, we just use
RegistryEnvironment.get() function to get the instance, which indeed
retrives the value from system property every time. If any test case
needs to run the test with other environment, it can just simply use
the existing SystemPropertyRule and RegistryEnvironment.${ENV}.setup
to change it.
This commit is contained in:
Shicong Huang 2019-06-26 16:57:19 -04:00 committed by GitHub
parent 33a7808e9e
commit 661e348fcb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 142 additions and 88 deletions

View file

@ -46,12 +46,14 @@ import google.registry.model.registry.Registry;
import google.registry.model.registry.Registry.TldType;
import google.registry.model.reporting.HistoryEntry;
import google.registry.testing.FakeResponse;
import google.registry.testing.SystemPropertyRule;
import google.registry.testing.mapreduce.MapreduceTestCase;
import java.util.Optional;
import java.util.Set;
import org.joda.money.Money;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -62,6 +64,9 @@ public class DeleteProberDataActionTest extends MapreduceTestCase<DeleteProberDa
private static final DateTime DELETION_TIME = DateTime.parse("2010-01-01T00:00:00.000Z");
@Rule
public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule();
@Before
public void init() {
// Entities in these two should not be touched.
@ -90,8 +95,8 @@ public class DeleteProberDataActionTest extends MapreduceTestCase<DeleteProberDa
action.response = new FakeResponse();
action.isDryRun = false;
action.tlds = ImmutableSet.of();
action.registryEnvironment = RegistryEnvironment.SANDBOX;
action.registryAdminClientId = "TheRegistrar";
RegistryEnvironment.SANDBOX.setup(systemPropertyRule);
}
private void runMapreduce() throws Exception {
@ -153,7 +158,7 @@ public class DeleteProberDataActionTest extends MapreduceTestCase<DeleteProberDa
@Test
public void testFail_givenNonDotTestTldOnProd() {
action.tlds = ImmutableSet.of("example");
action.registryEnvironment = RegistryEnvironment.PRODUCTION;
RegistryEnvironment.PRODUCTION.setup(systemPropertyRule);
IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, this::runMapreduce);
assertThat(thrown)

View file

@ -41,6 +41,7 @@ import google.registry.testing.AppEngineRule;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeResponse;
import google.registry.testing.SystemPropertyRule;
import google.registry.ui.server.SendEmailUtils;
import google.registry.util.EmailMessage;
import google.registry.util.SendEmailService;
@ -67,6 +68,9 @@ public final class ConsoleOteSetupActionTest {
@Rule public final MockitoRule mocks = MockitoJUnit.rule();
@Rule
public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule();
private final FakeResponse response = new FakeResponse();
private final ConsoleOteSetupAction action = new ConsoleOteSetupAction();
private final User user = new User("marla.singer@example.com", "gmail.com", "12345");
@ -87,7 +91,6 @@ public final class ConsoleOteSetupActionTest {
action.userService = UserServiceFactory.getUserService();
action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService);
action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false));
action.registryEnvironment = RegistryEnvironment.UNITTEST;
action.sendEmailUtils =
new SendEmailUtils(
new InternetAddress("outgoing@registry.example"),
@ -122,7 +125,7 @@ public final class ConsoleOteSetupActionTest {
@Test
public void testGet_authorized_onProduction() {
action.registryEnvironment = RegistryEnvironment.PRODUCTION;
RegistryEnvironment.PRODUCTION.setup(systemPropertyRule);
assertThrows(IllegalStateException.class, action::run);
}

View file

@ -42,6 +42,7 @@ import google.registry.testing.AppEngineRule;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeResponse;
import google.registry.testing.SystemPropertyRule;
import google.registry.ui.server.SendEmailUtils;
import google.registry.util.EmailMessage;
import google.registry.util.SendEmailService;
@ -69,6 +70,9 @@ public final class ConsoleRegistrarCreatorActionTest {
@Rule public final MockitoRule mocks = MockitoJUnit.rule();
@Rule
public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule();
private final FakeResponse response = new FakeResponse();
private final ConsoleRegistrarCreatorAction action = new ConsoleRegistrarCreatorAction();
private final User user = new User("marla.singer@example.com", "gmail.com", "12345");
@ -89,7 +93,6 @@ public final class ConsoleRegistrarCreatorActionTest {
action.userService = UserServiceFactory.getUserService();
action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService);
action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false));
action.registryEnvironment = RegistryEnvironment.UNITTEST;
action.sendEmailUtils =
new SendEmailUtils(
new InternetAddress("outgoing@registry.example"),
@ -142,7 +145,7 @@ public final class ConsoleRegistrarCreatorActionTest {
@Test
public void testGet_authorized_onProduction() {
action.registryEnvironment = RegistryEnvironment.PRODUCTION;
RegistryEnvironment.PRODUCTION.setup(systemPropertyRule);
action.run();
assertThat(response.getPayload()).contains("<h1>Create Registrar</h1>");
assertThat(response.getPayload()).contains("gtag('config', 'sampleId')");

View file

@ -28,7 +28,6 @@ import com.google.appengine.api.users.UserServiceFactory;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.net.MediaType;
import google.registry.config.RegistryEnvironment;
import google.registry.request.auth.AuthLevel;
import google.registry.request.auth.AuthResult;
import google.registry.request.auth.AuthenticatedRegistrarAccessor;
@ -52,10 +51,11 @@ import org.junit.runners.JUnit4;
public class ConsoleUiActionTest {
@Rule
public final AppEngineRule appEngineRule = AppEngineRule.builder()
.withDatastore()
.withUserService(UserInfo.create("marla.singer@example.com", "12345"))
.build();
public final AppEngineRule appEngineRule =
AppEngineRule.builder()
.withDatastore()
.withUserService(UserInfo.create("marla.singer@example.com", "12345"))
.build();
private final HttpServletRequest request = mock(HttpServletRequest.class);
private final FakeResponse response = new FakeResponse();
@ -79,7 +79,6 @@ public class ConsoleUiActionTest {
action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService);
action.paramClientId = Optional.empty();
action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false));
action.environment = RegistryEnvironment.UNITTEST;
action.analyticsConfig = ImmutableMap.of("googleAnalyticsId", "sampleId");
action.registrarAccessor =

View file

@ -36,6 +36,7 @@ import google.registry.model.registrar.Registrar;
import google.registry.request.auth.AuthenticatedRegistrarAccessor;
import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
import google.registry.testing.CertificateSamples;
import google.registry.testing.SystemPropertyRule;
import google.registry.testing.TaskQueueHelper.TaskMatcher;
import google.registry.util.CidrAddressBlock;
import google.registry.util.EmailMessage;
@ -44,6 +45,7 @@ import java.util.function.BiFunction;
import java.util.function.Function;
import org.json.simple.JSONValue;
import org.json.simple.parser.ParseException;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -53,6 +55,9 @@ import org.mockito.ArgumentCaptor;
@RunWith(JUnit4.class)
public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase {
@Rule
public final SystemPropertyRule systemPropertyRule = new SystemPropertyRule();
@Test
public void testSuccess_updateRegistrarInfo_andSendsNotificationEmail() throws Exception {
String expectedEmailBody = loadFile(getClass(), "update_registrar_email.txt");
@ -393,7 +398,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
@Test
public void testUpdate_allowedTlds_failedWhenNoWhoisAbuseContactExists() {
setUserAdmin();
action.registryEnvironment = RegistryEnvironment.PRODUCTION;
RegistryEnvironment.PRODUCTION.setup(systemPropertyRule);
Map<String, Object> args = Maps.newHashMap(loadRegistrar(CLIENT_ID).toJsonMap());
args.put("allowedTlds", ImmutableList.of("newtld", "currenttld"));

View file

@ -33,7 +33,6 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.truth.Truth;
import google.registry.config.RegistryEnvironment;
import google.registry.model.ofy.Ofy;
import google.registry.model.registrar.RegistrarContact;
import google.registry.request.JsonActionRunner;
@ -116,7 +115,6 @@ public abstract class RegistrarSettingsActionTestCase {
getGSuiteOutgoingEmailDisplayName(),
ImmutableList.of("notification@test.example", "notification2@test.example"),
emailService);
action.registryEnvironment = RegistryEnvironment.get();
action.registrarConsoleMetrics = new RegistrarConsoleMetrics();
action.authResult =
AuthResult.create(