Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade App Engine and webserver tests from JUnit 4 to 5 #720

Merged
merged 8 commits into from Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions config/presubmits.py
Expand Up @@ -93,20 +93,20 @@ def fails(self, file):
PresubmitCheck(
r".*\bSystem\.(out|err)\.print", "java", {
"StackdriverDashboardBuilder.java", "/tools/", "/example/",
"RegistryTestServerMain.java", "TestServerRule.java",
"RegistryTestServerMain.java", "TestServerExtension.java",
"FlowDocumentationTool.java"
}):
"System.(out|err).println is only allowed in tools/ packages. Please "
"use a logger instead.",

# ObjectifyService.register is restricted to main/ or AppEngineRule.
# ObjectifyService.register is restricted to main/ or AppEngineExtension.
PresubmitCheck(
r".*\bObjectifyService\.register", "java", {
"/build/", "/generated/", "node_modules/", "src/main/",
"AppEngineRule.java"
"AppEngineExtension.java"
}):
"ObjectifyService.register is not allowed in tests. Please use "
"AppengineRule.register instead.",
"ObjectifyService.register(...) is not allowed in tests. Please use "
"AppEngineExtension.register(...) instead.",

# PostgreSQLContainer instantiation must specify docker tag
PresubmitCheck(
Expand Down
1 change: 1 addition & 0 deletions core/build.gradle
Expand Up @@ -315,6 +315,7 @@ dependencies {
testCompile deps['org.junit.jupiter:junit-jupiter-engine']
testCompile deps['org.junit.jupiter:junit-jupiter-migrationsupport']
testCompile deps['org.junit.jupiter:junit-jupiter-params']
testCompile deps['org.junit-pioneer:junit-pioneer']
testCompile deps['org.junit.platform:junit-platform-runner']
testCompile deps['org.junit.platform:junit-platform-suite-api']
testCompile deps['org.junit.vintage:junit-vintage-engine']
Expand Down
1 change: 1 addition & 0 deletions core/gradle/dependency-locks/testCompile.lockfile
Expand Up @@ -243,6 +243,7 @@ org.jboss:jandex:2.1.3.Final
org.jetbrains:annotations:19.0.0
org.joda:joda-money:1.0.1
org.json:json:20160810
org.junit-pioneer:junit-pioneer:0.7.0
org.junit.jupiter:junit-jupiter-api:5.6.2
org.junit.jupiter:junit-jupiter-engine:5.6.2
org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
Expand Down
1 change: 1 addition & 0 deletions core/gradle/dependency-locks/testCompileClasspath.lockfile
Expand Up @@ -241,6 +241,7 @@ org.jboss:jandex:2.1.3.Final
org.jetbrains:annotations:19.0.0
org.joda:joda-money:1.0.1
org.json:json:20160810
org.junit-pioneer:junit-pioneer:0.7.0
org.junit.jupiter:junit-jupiter-api:5.6.2
org.junit.jupiter:junit-jupiter-engine:5.6.2
org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
Expand Down
1 change: 1 addition & 0 deletions core/gradle/dependency-locks/testRuntime.lockfile
Expand Up @@ -246,6 +246,7 @@ org.jboss:jandex:2.1.3.Final
org.jetbrains:annotations:19.0.0
org.joda:joda-money:1.0.1
org.json:json:20160810
org.junit-pioneer:junit-pioneer:0.7.0
org.junit.jupiter:junit-jupiter-api:5.6.2
org.junit.jupiter:junit-jupiter-engine:5.6.2
org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
Expand Down
1 change: 1 addition & 0 deletions core/gradle/dependency-locks/testRuntimeClasspath.lockfile
Expand Up @@ -246,6 +246,7 @@ org.jboss:jandex:2.1.3.Final
org.jetbrains:annotations:19.0.0
org.joda:joda-money:1.0.1
org.json:json:20160810
org.junit-pioneer:junit-pioneer:0.7.0
org.junit.jupiter:junit-jupiter-api:5.6.2
org.junit.jupiter:junit-jupiter-engine:5.6.2
org.junit.jupiter:junit-jupiter-migrationsupport:5.6.2
Expand Down
Expand Up @@ -60,8 +60,8 @@ public interface InitSqlPipelineOptions extends GcpOptions {

void setEnvironment(String environment);

@Description("The GCP project that contains the keyring used for decrypting the "
+ "SQL credential file.")
@Description(
"The GCP project that contains the keyring used for decrypting the " + "SQL credential file.")
@Nullable
String getCloudKmsProjectId();

Expand Down
Expand Up @@ -27,18 +27,40 @@
@Parameters(commandDescription = "Deploy the Spec11 pipeline to GCS.")
public class DeploySpec11PipelineCommand implements Command {

@Inject @Config("projectId") String projectId;
@Inject @Config("beamStagingUrl") String beamStagingUrl;
@Inject @Config("spec11TemplateUrl")String spec11TemplateUrl;
@Inject @Config("reportingBucketUrl")String reportingBucketUrl;
@Inject
@Config("projectId")
String projectId;

@Inject
@Config("beamStagingUrl")
String beamStagingUrl;

@Inject
@Config("spec11TemplateUrl")
String spec11TemplateUrl;

@Inject
@Config("reportingBucketUrl")
String reportingBucketUrl;

@Inject @LocalCredential GoogleCredentialsBundle googleCredentialsBundle;
@Inject Retrier retrier;
@Inject @Nullable @Config("sqlAccessInfoFile") String sqlAccessInfoFile;

@Inject
@Nullable
@Config("sqlAccessInfoFile")
String sqlAccessInfoFile;

@Override
public void run() {
Spec11Pipeline pipeline = new Spec11Pipeline(projectId, beamStagingUrl, spec11TemplateUrl,
reportingBucketUrl, googleCredentialsBundle, retrier);
Spec11Pipeline pipeline =
new Spec11Pipeline(
projectId,
beamStagingUrl,
spec11TemplateUrl,
reportingBucketUrl,
googleCredentialsBundle,
retrier);
pipeline.deploy();
}
}
7 changes: 3 additions & 4 deletions core/src/main/java/google/registry/tools/DomainLockUtils.java
Expand Up @@ -303,17 +303,16 @@ private static void verifyDomainNotLocked(DomainBase domainBase, boolean isAdmin

private static void verifyDomainLocked(DomainBase domainBase, boolean isAdmin) {
checkArgument(
isAdmin || !Sets.intersection(domainBase.getStatusValues(), REGISTRY_LOCK_STATUSES)
.isEmpty(),
isAdmin
|| !Sets.intersection(domainBase.getStatusValues(), REGISTRY_LOCK_STATUSES).isEmpty(),
"Domain %s is already unlocked",
domainBase.getDomainName());
}

private DomainBase getDomain(String domainName, String registrarId, DateTime now) {
DomainBase domain =
loadByForeignKeyCached(DomainBase.class, domainName, now)
.orElseThrow(
() -> new IllegalArgumentException("Domain doesn't exist"));
.orElseThrow(() -> new IllegalArgumentException("Domain doesn't exist"));
// The user must have specified either the correct registrar ID or the admin registrar ID
checkArgument(
registryAdminRegistrarId.equals(registrarId)
Expand Down
Expand Up @@ -34,9 +34,7 @@
import java.util.List;
import javax.inject.Inject;

/**
* Shared base class for commands to registry lock or unlock a domain via EPP.
*/
/** Shared base class for commands to registry lock or unlock a domain via EPP. */
public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand
implements CommandWithRemoteApi {

Expand All @@ -61,8 +59,7 @@ public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand
@Config("registryAdminClientId")
String registryAdminClientId;

@Inject
DomainLockUtils domainLockUtils;
@Inject DomainLockUtils domainLockUtils;

protected ImmutableSet<String> getDomains() {
return ImmutableSet.copyOf(mainParameters);
Expand All @@ -88,25 +85,28 @@ protected String execute() {
.forEach(
batch ->
// we require that the jpaTm is the outer transaction in DomainLockUtils
jpaTm().transact(() -> tm().transact(
() -> {
for (String domain : batch) {
try {
createAndApplyRequest(domain);
} catch (Throwable t) {
logger.atSevere().withCause(t).log(
"Error when (un)locking domain %s.", domain);
failedDomainsToReasons.put(domain, t.getMessage());
continue;
}
successfulDomainsBuilder.add(domain);
}
})));
jpaTm()
.transact(
() ->
tm().transact(
() -> {
for (String domain : batch) {
try {
createAndApplyRequest(domain);
} catch (Throwable t) {
logger.atSevere().withCause(t).log(
"Error when (un)locking domain %s.", domain);
failedDomainsToReasons.put(domain, t.getMessage());
continue;
}
successfulDomainsBuilder.add(domain);
}
})));
ImmutableSet<String> successfulDomains = successfulDomainsBuilder.build();
ImmutableSet<String> failedDomains = failedDomainsToReasons.build().entrySet()
.stream()
.map(entry -> String.format("%s (%s)", entry.getKey(), entry.getValue()))
.collect(toImmutableSet());
ImmutableSet<String> failedDomains =
failedDomainsToReasons.build().entrySet().stream()
.map(entry -> String.format("%s (%s)", entry.getKey(), entry.getValue()))
.collect(toImmutableSet());
return String.format(
"Successfully locked/unlocked domains:\n%s\nFailed domains:\n%s",
successfulDomains, failedDomains);
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/google/registry/tools/RegistryCli.java
Expand Up @@ -68,8 +68,9 @@ final class RegistryCli implements AutoCloseable, CommandRunner {

@Parameter(
names = {"--sql_access_info"},
description = "Name of a file containing space-separated SQL access info used when deploying "
+ "Beam pipelines")
description =
"Name of a file containing space-separated SQL access info used when deploying "
+ "Beam pipelines")
private String sqlAccessInfoFile = null;

// Do not make this final - compile-time constant inlining may interfere with JCommander.
Expand Down
Expand Up @@ -36,6 +36,7 @@
*/
@Parameters(separators = " =", commandDescription = "Generate PostgreSQL schema.")
public class GenerateSqlSchemaCommand implements Command {

private static final String DB_NAME = "postgres";
private static final String DB_USERNAME = "postgres";
private static final String DB_PASSWORD = "domain-registry";
Expand Down
Expand Up @@ -27,7 +27,7 @@
import com.google.common.collect.ImmutableMap;
import google.registry.model.ofy.CommitLogCheckpoint;
import google.registry.model.ofy.CommitLogCheckpointRoot;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock;
import google.registry.testing.TaskQueueHelper.TaskMatcher;
import google.registry.util.Retrier;
Expand All @@ -43,8 +43,8 @@ public class CommitLogCheckpointActionTest {
private static final String QUEUE_NAME = "export-commits";

@RegisterExtension
public final AppEngineRule appEngine =
AppEngineRule.builder().withDatastoreAndCloudSql().withTaskQueue().build();
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build();

private CommitLogCheckpointStrategy strategy = mock(CommitLogCheckpointStrategy.class);

Expand Down
Expand Up @@ -31,7 +31,7 @@
import google.registry.model.registry.Registry;
import google.registry.persistence.transaction.TransactionManager;
import google.registry.schema.cursor.CursorDao;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock;
import google.registry.testing.InjectRule;
import org.joda.time.DateTime;
Expand All @@ -44,7 +44,8 @@
public class CommitLogCheckpointStrategyTest {

@RegisterExtension
public final AppEngineRule appEngine = AppEngineRule.builder().withDatastoreAndCloudSql().build();
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().build();

@RegisterExtension public final InjectRule inject = new InjectRule();

Expand Down
Expand Up @@ -34,7 +34,7 @@
import google.registry.model.ofy.CommitLogCheckpoint;
import google.registry.model.ofy.CommitLogManifest;
import google.registry.model.ofy.CommitLogMutation;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.GcsTestingUtils;
import google.registry.testing.TestObject;
import java.util.List;
Expand All @@ -47,8 +47,8 @@
public class ExportCommitLogDiffActionTest {

@RegisterExtension
public final AppEngineRule appEngine =
AppEngineRule.builder()
public final AppEngineExtension appEngine =
AppEngineExtension.builder()
.withDatastoreAndCloudSql()
.withOfyTestEntities(TestObject.class)
.build();
Expand Down
Expand Up @@ -34,7 +34,7 @@
import com.google.common.collect.Iterators;
import com.google.common.flogger.LoggerConfig;
import com.google.common.testing.TestLogHandler;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import java.io.IOException;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
Expand All @@ -59,7 +59,8 @@ public class GcsDiffFileListerTest {
private final TestLogHandler logHandler = new TestLogHandler();

@RegisterExtension
public final AppEngineRule appEngine = AppEngineRule.builder().withDatastoreAndCloudSql().build();
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().build();

@BeforeEach
void beforeEach() throws Exception {
Expand Down
Expand Up @@ -42,7 +42,7 @@
import google.registry.model.ofy.CommitLogCheckpointRoot;
import google.registry.model.ofy.CommitLogManifest;
import google.registry.model.ofy.CommitLogMutation;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeSleeper;
import google.registry.testing.TestObject;
Expand All @@ -68,8 +68,8 @@ public class RestoreCommitLogsActionTest {
private final GcsService gcsService = createGcsService();

@RegisterExtension
public final AppEngineRule appEngine =
AppEngineRule.builder()
public final AppEngineExtension appEngine =
AppEngineExtension.builder()
.withDatastoreAndCloudSql()
.withOfyTestEntities(TestObject.class)
.build();
Expand Down
Expand Up @@ -39,7 +39,7 @@
import com.googlecode.objectify.Key;
import google.registry.model.contact.ContactResource;
import google.registry.schema.domain.RegistryLock;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeSleeper;
import google.registry.testing.InjectRule;
Expand All @@ -64,8 +64,8 @@
public class AsyncTaskEnqueuerTest {

@RegisterExtension
public final AppEngineRule appEngine =
AppEngineRule.builder().withDatastoreAndCloudSql().withTaskQueue().build();
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build();

@RegisterExtension public final InjectRule inject = new InjectRule();

Expand Down
Expand Up @@ -34,7 +34,7 @@
import google.registry.model.domain.DomainBase;
import google.registry.model.host.HostResource;
import google.registry.schema.domain.RegistryLock;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeResponse;
Expand Down Expand Up @@ -65,8 +65,8 @@ public class RelockDomainActionTest {
mock(AppEngineServiceUtils.class), clock, Duration.ZERO));

@RegisterExtension
public final AppEngineRule appEngineRule =
AppEngineRule.builder()
public final AppEngineExtension appEngineRule =
AppEngineExtension.builder()
.withDatastoreAndCloudSql()
.withUserService(UserInfo.create(POC_ID, "12345"))
.build();
Expand Down
Expand Up @@ -42,7 +42,7 @@
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.ofy.Ofy;
import google.registry.request.Response;
import google.registry.testing.AppEngineRule;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock;
import google.registry.testing.InjectRule;
import google.registry.testing.TaskQueueHelper.TaskMatcher;
Expand All @@ -63,8 +63,8 @@
public class ResaveEntityActionTest {

@RegisterExtension
public final AppEngineRule appEngine =
AppEngineRule.builder().withDatastoreAndCloudSql().withTaskQueue().build();
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build();

@RegisterExtension public final InjectRule inject = new InjectRule();

Expand Down