From 73f7e185f2274c72ed6681ba55b4214337f15e05 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 16 May 2019 09:41:11 -0700 Subject: [PATCH] Migrate users from the old, deprecated Subject.fail* methods to the new Subject.fail* methods or, in some cases, to Subject.check. Most of the changes in this CL were made manually. I've tried to preserve all information (and of course behavior!), but the format and grammar of the messages does change. For example before-and-after messages, see the LSC doc. In some of the CLs in this round (e.g., jscomp), I don't know a lot about the domain being tested, and the assertions are complex, so please let me know if my new phrasing is wrong or confusing. Thanks again for your patience with all the Truth changes lately. END_PUBLIC More information: [] Tested: TAP for global presubmit queue [] ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=248543745 --- .../testing/AbstractEppResourceSubject.java | 58 ++++++------------- .../registry/testing/DomainBaseSubject.java | 15 ++--- .../registry/testing/EppExceptionSubject.java | 2 +- .../registry/testing/EppMetricSubject.java | 20 +++---- .../registry/testing/HistoryEntrySubject.java | 34 ++++------- 5 files changed, 43 insertions(+), 86 deletions(-) diff --git a/javatests/google/registry/testing/AbstractEppResourceSubject.java b/javatests/google/registry/testing/AbstractEppResourceSubject.java index 66a6f8e8b..472d692fa 100644 --- a/javatests/google/registry/testing/AbstractEppResourceSubject.java +++ b/javatests/google/registry/testing/AbstractEppResourceSubject.java @@ -16,6 +16,7 @@ package google.registry.testing; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.lenientFormat; +import static com.google.common.truth.Fact.fact; import static com.google.common.truth.Fact.simpleFact; import static google.registry.model.EppResourceUtils.isActive; import static google.registry.testing.DatastoreHelper.getHistoryEntriesOfType; @@ -32,7 +33,6 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.testing.TruthChainer.And; import google.registry.testing.TruthChainer.Which; import java.util.List; -import java.util.Objects; import javax.annotation.Nullable; import org.joda.time.DateTime; @@ -69,14 +69,14 @@ abstract class AbstractEppResourceSubject String diffText = prettyPrintEntityDeepDiff( ((ImmutableObject) other).toDiffableFieldMap(), actual.toDiffableFieldMap()); - fail(String.format("is equal to %s\n\nIt differs as follows:\n%s", other, diffText)); + failWithoutActual(fact("expected", other), fact("but was", actual), fact("diff", diffText)); } // Otherwise, fall back to regular behavior. super.isEqualTo(other); } public And hasRepoId(long roid) { - return hasValue(roid, actual.getRepoId(), "has repoId"); + return hasValue(roid, actual.getRepoId(), "getRepoId()"); } public And hasNoHistoryEntries() { @@ -87,20 +87,13 @@ abstract class AbstractEppResourceSubject } public And hasNumHistoryEntries(int num) { - if (getHistoryEntries().size() != num) { - failWithBadResults("has this number of history entries", num, getHistoryEntries().size()); - } + check("getHistoryEntries()").that(getHistoryEntries()).hasSize(num); return andChainer(); } public And hasNumHistoryEntriesOfType(HistoryEntry.Type type, int num) { List entries = getHistoryEntriesOfType(actual, type); - if (entries.size() != num) { - failWithBadResults( - String.format("has this number of history entries of type %s", type.toString()), - num, - entries.size()); - } + check("getHistoryEntriesOfType(%s)", type).that(entries).hasSize(num); return andChainer(); } @@ -113,11 +106,7 @@ abstract class AbstractEppResourceSubject } public And hasOnlyOneHistoryEntry() { - int numHistoryEntries = getHistoryEntries().size(); - if (numHistoryEntries != 1) { - fail(String.format("has exactly one history entry (it has %d)", numHistoryEntries)); - } - return andChainer(); + return hasNumHistoryEntries(1); } public HistoryEntrySubject hasOnlyOneHistoryEntryWhich() { @@ -128,10 +117,7 @@ abstract class AbstractEppResourceSubject public Which hasHistoryEntryAtIndex(int index) { List historyEntries = getHistoryEntries(); - if (historyEntries.size() < index + 1) { - failWithBadResults( - "has at least number of history entries", index + 1, historyEntries.size()); - } + check("getHistoryEntries().size()").that(historyEntries.size()).isAtLeast(index + 1); return new Which<>(assertAboutHistoryEntries() .that(getHistoryEntries().get(index)).withCustomDisplaySubject(String.format( "the history entry for %s at index %s", actualAsString(), index))); @@ -166,7 +152,7 @@ abstract class AbstractEppResourceSubject } public And hasDeletionTime(DateTime deletionTime) { - return hasValue(deletionTime, actual.getDeletionTime(), "has deletionTime"); + return hasValue(deletionTime, actual.getDeletionTime(), "getDeletionTime()"); } public And hasLastEppUpdateTime(DateTime lastUpdateTime) { @@ -175,14 +161,12 @@ abstract class AbstractEppResourceSubject public And hasLastEppUpdateTimeAtLeast(DateTime before) { DateTime lastEppUpdateTime = actual.getLastEppUpdateTime(); - if (lastEppUpdateTime == null || before.isAfter(lastEppUpdateTime)) { - failWithBadResults("has lastEppUpdateTime at least", before, lastEppUpdateTime); - } + check("getLastEppUpdateTime()").that(lastEppUpdateTime).isAtLeast(before); return andChainer(); } public And hasLastEppUpdateClientId(String clientId) { - return hasValue(clientId, actual.getLastEppUpdateClientId(), "has lastEppUpdateClientId"); + return hasValue(clientId, actual.getLastEppUpdateClientId(), "getLastEppUpdateClientId()"); } @@ -190,38 +174,30 @@ abstract class AbstractEppResourceSubject return hasValue( clientId, actual.getPersistedCurrentSponsorClientId(), - "has persisted currentSponsorClientId"); + "getPersistedCurrentSponsorClientId()"); } public And isActiveAt(DateTime time) { if (!isActive(actual, time)) { - fail("is active at " + time); + failWithActual("expected to be active at", time); } return andChainer(); } public And isNotActiveAt(DateTime time) { if (isActive(actual, time)) { - fail("is not active at " + time); + failWithActual("expected not to be active at", time); } return andChainer(); } - protected void failWithBadResults(String dualVerb, Object expected, Object actual) { - failWithBadResults(dualVerb, expected, dualVerb, actual); - } - - protected And hasValue(E expected, E actual, String verb) { - if (!Objects.equals(expected, actual)) { - failWithBadResults(verb, expected, actual); - } + protected And hasValue(E expected, E actual, String name) { + check(name).that(actual).isEqualTo(expected); return andChainer(); } - protected And doesNotHaveValue(E badValue, E actual, String valueName) { - if (Objects.equals(badValue, actual)) { - fail("has " + valueName + " not equal to " + badValue); - } + protected And doesNotHaveValue(E badValue, E actual, String name) { + check(name).that(actual).isNotEqualTo(badValue); return andChainer(); } } diff --git a/javatests/google/registry/testing/DomainBaseSubject.java b/javatests/google/registry/testing/DomainBaseSubject.java index a2c7ab21a..25196a2a0 100644 --- a/javatests/google/registry/testing/DomainBaseSubject.java +++ b/javatests/google/registry/testing/DomainBaseSubject.java @@ -26,7 +26,6 @@ import google.registry.model.domain.launch.LaunchNotice; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.AuthInfo; import google.registry.testing.TruthChainer.And; -import java.util.Objects; import java.util.Set; import org.joda.time.DateTime; @@ -74,19 +73,17 @@ public final class DomainBaseSubject } public And hasRegistrationExpirationTime(DateTime expiration) { - if (!Objects.equals(actual.getRegistrationExpirationTime(), expiration)) { - failWithBadResults( - "has registrationExpirationTime", expiration, actual.getRegistrationExpirationTime()); - } - return andChainer(); + return hasValue( + expiration, actual.getRegistrationExpirationTime(), "getRegistrationExpirationTime()"); } public And hasLastTransferTime(DateTime lastTransferTime) { - return hasValue(lastTransferTime, actual.getLastTransferTime(), "has lastTransferTime"); + return hasValue(lastTransferTime, actual.getLastTransferTime(), "getLastTransferTime()"); } public And hasLastTransferTimeNotEqualTo(DateTime lastTransferTime) { - return doesNotHaveValue(lastTransferTime, actual.getLastTransferTime(), "lastTransferTime"); + return doesNotHaveValue( + lastTransferTime, actual.getLastTransferTime(), "getLastTransferTime()"); } public And hasDeletePollMessage() { @@ -104,7 +101,7 @@ public final class DomainBaseSubject } public And hasSmdId(String smdId) { - return hasValue(smdId, actual.getSmdId(), "has smdId"); + return hasValue(smdId, actual.getSmdId(), "getSmdId()"); } public static SimpleSubjectBuilder assertAboutDomains() { diff --git a/javatests/google/registry/testing/EppExceptionSubject.java b/javatests/google/registry/testing/EppExceptionSubject.java index 584db2f8a..f49764dec 100644 --- a/javatests/google/registry/testing/EppExceptionSubject.java +++ b/javatests/google/registry/testing/EppExceptionSubject.java @@ -60,7 +60,7 @@ public class EppExceptionSubject extends Subject(this); } diff --git a/javatests/google/registry/testing/EppMetricSubject.java b/javatests/google/registry/testing/EppMetricSubject.java index 204c369f4..916898ea9 100644 --- a/javatests/google/registry/testing/EppMetricSubject.java +++ b/javatests/google/registry/testing/EppMetricSubject.java @@ -15,6 +15,7 @@ package google.registry.testing; import static com.google.common.truth.Fact.simpleFact; +import static com.google.common.truth.OptionalSubject.optionals; import static com.google.common.truth.Truth.assertAbout; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; @@ -23,7 +24,6 @@ import com.google.common.truth.Subject; import google.registry.model.eppoutput.Result.Code; import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.TruthChainer.And; -import java.util.Objects; import java.util.Optional; /** Utility methods for asserting things about {@link EppMetric} instances. */ @@ -41,15 +41,15 @@ public class EppMetricSubject extends Subject { } public And hasClientId(String clientId) { - return hasValue(clientId, actual.getClientId(), "has clientId"); + return hasValue(clientId, actual.getClientId(), "getClientId()"); } public And hasCommandName(String commandName) { - return hasValue(commandName, actual.getCommandName(), "has commandName"); + return hasValue(commandName, actual.getCommandName(), "getCommandName()"); } public And hasStatus(Code status) { - return hasValue(status, actual.getStatus(), "has status"); + return hasValue(status, actual.getStatus(), "getStatus()"); } public And hasNoStatus() { @@ -60,7 +60,7 @@ public class EppMetricSubject extends Subject { } public And hasTld(String tld) { - return hasValue(tld, actual.getTld(), "has tld"); + return hasValue(tld, actual.getTld(), "getTld()"); } public And hasNoTld() { @@ -70,15 +70,9 @@ public class EppMetricSubject extends Subject { return new And<>(this); } - private And hasValue(E expected, Optional actual, String verb) { + private And hasValue(E expected, Optional actual, String name) { checkArgumentNotNull(expected, "Expected value cannot be null"); - if (actual == null) { - failWithActual("expected to be non-null", expected); - } else if (!actual.isPresent()) { - failWithActual("expected to have value", expected); - } else if (!Objects.equals(expected, actual.get())) { - failWithBadResults(verb, expected, verb, actual); - } + check(name).about(optionals()).that(actual).hasValue(expected); return new And<>(this); } diff --git a/javatests/google/registry/testing/HistoryEntrySubject.java b/javatests/google/registry/testing/HistoryEntrySubject.java index debfb1757..1cdc0754a 100644 --- a/javatests/google/registry/testing/HistoryEntrySubject.java +++ b/javatests/google/registry/testing/HistoryEntrySubject.java @@ -23,7 +23,6 @@ import com.google.common.truth.Subject; import google.registry.model.domain.Period; import google.registry.model.reporting.HistoryEntry; import google.registry.testing.TruthChainer.And; -import java.util.Objects; import java.util.Optional; import org.joda.time.DateTime; @@ -49,23 +48,23 @@ public class HistoryEntrySubject extends Subject hasType(HistoryEntry.Type type) { - return hasValue(type, actual.getType(), "has type"); + return hasValue(type, actual.getType(), "getType()"); } public And hasClientId(String clientId) { - return hasValue(clientId, actual.getClientId(), "has client ID"); + return hasValue(clientId, actual.getClientId(), "getClientId()"); } public And hasOtherClientId(String otherClientId) { - return hasValue(otherClientId, actual.getOtherClientId(), "has other client ID"); + return hasValue(otherClientId, actual.getOtherClientId(), "getOtherClientId()"); } public And hasModificationTime(DateTime modificationTime) { - return hasValue(modificationTime, actual.getModificationTime(), "has modification time"); + return hasValue(modificationTime, actual.getModificationTime(), "getModificationTime()"); } public And bySuperuser(boolean superuser) { - return hasValue(superuser, actual.getBySuperuser(), "has modification time"); + return hasValue(superuser, actual.getBySuperuser(), "getBySuperuser()"); } public And hasPeriod() { @@ -78,9 +77,9 @@ public class HistoryEntrySubject extends Subject hasPeriodYears(int years) { return hasPeriod() .and() - .hasValue(Period.Unit.YEARS, actual.getPeriod().getUnit(), "has period in") + .hasValue(Period.Unit.YEARS, actual.getPeriod().getUnit(), "getPeriod().getUnit()") .and() - .hasValue(years, actual.getPeriod().getValue(), "has period length"); + .hasValue(years, actual.getPeriod().getValue(), "getPeriod().getValue()"); } public And hasNoXml() { @@ -91,26 +90,17 @@ public class HistoryEntrySubject extends Subject hasMetadataReason(String reason) { - return hasValue(reason, actual.getReason(), "has metadata reason"); + return hasValue(reason, actual.getReason(), "getReason()"); } public And hasMetadataRequestedByRegistrar( boolean requestedByRegistrar) { - if (actual.getRequestedByRegistrar() != requestedByRegistrar) { - failWithActual( - "expected to have metadata requestedByRegistrar with value", requestedByRegistrar); - } - return new And<>(this); + return hasValue( + requestedByRegistrar, actual.getRequestedByRegistrar(), "getRequestedByRegistrar()"); } - protected void failWithBadResults(String dualVerb, Object expected, Object actual) { - failWithBadResults(dualVerb, expected, dualVerb, actual); - } - - protected And hasValue(E expected, E actual, String verb) { - if (!Objects.equals(expected, actual)) { - failWithBadResults(verb, expected, actual); - } + protected And hasValue(E expected, E actual, String name) { + check(name).that(actual).isEqualTo(expected); return new And<>(this); }