Skip to content

Commit

Permalink
Fix some misconfigured collections test suites that were passing due to
Browse files Browse the repository at this point in the history
#7401.

RELNOTES=n/a
PiperOrigin-RevId: 681498058
  • Loading branch information
chaoren authored and Google Java Core Libraries committed Oct 2, 2024
1 parent f640a0f commit 5da71a7
Show file tree
Hide file tree
Showing 16 changed files with 60 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@
import com.google.common.collect.testing.DerivedCollectionGenerators.SortedSetSubsetTestSetGenerator;
import com.google.common.collect.testing.features.CollectionFeature;
import com.google.common.collect.testing.features.Feature;
import com.google.common.collect.testing.testers.CollectionAddAllTester;
import com.google.common.collect.testing.testers.CollectionAddTester;
import com.google.common.collect.testing.testers.SortedSetNavigationTester;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import junit.framework.TestSuite;

/**
Expand Down Expand Up @@ -87,13 +92,20 @@ final TestSuite createSubsetSuite(
(TestSortedSetGenerator<E>) parentBuilder.getSubjectGenerator().getInnerGenerator();

List<Feature<?>> features = new ArrayList<>(parentBuilder.getFeatures());
features.remove(CollectionFeature.ALLOWS_NULL_VALUES);
Set<Method> suppressing = new HashSet<>(parentBuilder.getSuppressedTests());
features.add(CollectionFeature.SUBSET_VIEW);
if (features.remove(CollectionFeature.ALLOWS_NULL_VALUES)) {
// the null value might be out of bounds, so we can't always construct a subset with nulls
features.add(CollectionFeature.ALLOWS_NULL_QUERIES);
// but add null might still be supported if it happens to be within range of the subset
suppressing.add(CollectionAddTester.getAddNullUnsupportedMethod());
suppressing.add(CollectionAddAllTester.getAddAllNullUnsupportedMethod());
}

return newBuilderUsing(delegate, to, from)
.named(parentBuilder.getName() + " subSet " + from + "-" + to)
.withFeatures(features)
.suppressing(parentBuilder.getSuppressedTests())
.suppressing(suppressing)
.withSetUp(parentBuilder.getSetUp())
.withTearDown(parentBuilder.getTearDown())
.createTestSuite();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.GwtCompatible;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
Expand All @@ -38,6 +40,7 @@ public static class ImmutableBiMapGenerator extends TestStringBiMapGenerator {
protected BiMap<String, String> create(Entry<String, String>[] entries) {
ImmutableBiMap.Builder<String, String> builder = ImmutableBiMap.builder();
for (Entry<String, String> entry : entries) {
checkNotNull(entry);
builder.put(entry.getKey(), entry.getValue());
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.GwtCompatible;
import com.google.common.collect.BiMap;
import com.google.common.collect.testing.DerivedGenerator;
Expand Down Expand Up @@ -112,6 +114,7 @@ public SampleElements<Entry<V, K>> samples() {
}

private Entry<V, K> reverse(Entry<K, V> entry) {
checkNotNull(entry);
return Helpers.mapEntry(entry.getValue(), entry.getKey());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ Set<Feature<?>> computeMultimapGetFeatures(Set<Feature<?>> multimapFeatures) {
if (derivedFeatures.contains(CollectionFeature.SUPPORTS_ADD)) {
derivedFeatures.add(ListFeature.SUPPORTS_ADD_WITH_INDEX);
}
if (derivedFeatures.contains(CollectionFeature.SUPPORTS_REMOVE)) {
derivedFeatures.add(ListFeature.SUPPORTS_REMOVE_WITH_INDEX);
}
if (derivedFeatures.contains(CollectionFeature.GENERAL_PURPOSE)) {
derivedFeatures.add(ListFeature.GENERAL_PURPOSE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.testing.Helpers.mapEntry;

import com.google.common.annotations.GwtCompatible;
Expand Down Expand Up @@ -53,6 +54,7 @@ public static class ImmutableMapGenerator extends TestStringMapGenerator {
protected Map<String, String> create(Entry<String, String>[] entries) {
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
for (Entry<String, String> entry : entries) {
checkNotNull(entry);
builder.put(entry.getKey(), entry.getValue());
}
return builder.buildOrThrow();
Expand Down Expand Up @@ -142,7 +144,7 @@ public List<Entry<String, Integer>> create(Object... elements) {
ImmutableMap.Builder<String, Integer> builder = ImmutableMap.builder();
for (Object o : elements) {
@SuppressWarnings("unchecked")
Entry<String, Integer> entry = (Entry<String, Integer>) o;
Entry<String, Integer> entry = (Entry<String, Integer>) checkNotNull(o);
builder.put(entry);
}
return builder.buildOrThrow().entrySet().asList();
Expand All @@ -154,7 +156,7 @@ public static class ImmutableEnumMapGenerator extends TestEnumMapGenerator {
protected Map<AnEnum, String> create(Entry<AnEnum, String>[] entries) {
Map<AnEnum, String> map = Maps.newHashMap();
for (Entry<AnEnum, String> entry : entries) {
// checkArgument(!map.containsKey(entry.getKey()));
checkNotNull(entry);
map.put(entry.getKey(), entry.getValue());
}
return Maps.immutableEnumMap(map);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void testPutAllOnPresentNullKey() {
assertGet(null, v3(), v4());
}

@MapFeature.Require(absent = ALLOWS_NULL_KEYS)
@MapFeature.Require(value = SUPPORTS_PUT, absent = ALLOWS_NULL_KEYS)
public void testPutAllNullForbidden() {
try {
multimap().putAll(null, Collections.singletonList(v3()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public List<Entry<String, Integer>> create(Object... elements) {
ImmutableSortedMap.Builder<String, Integer> builder = ImmutableSortedMap.naturalOrder();
for (Object o : elements) {
@SuppressWarnings("unchecked")
Entry<String, Integer> entry = (Entry<String, Integer>) o;
Entry<String, Integer> entry = (Entry<String, Integer>) checkNotNull(o);
builder.put(entry);
}
return builder.build().entrySet().asList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static Test suite() {
CollectionSize.ANY,
CollectionFeature.KNOWN_ORDER,
CollectionFeature.SERIALIZABLE,
CollectionFeature.ALLOWS_NULL_QUERIES)
CollectionFeature.ALLOWS_NULL_VALUES)
.named("Multisets.unmodifiableMultiset[LinkedHashMultiset]")
.createTestSuite());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@
import com.google.common.collect.testing.DerivedCollectionGenerators.SortedSetSubsetTestSetGenerator;
import com.google.common.collect.testing.features.CollectionFeature;
import com.google.common.collect.testing.features.Feature;
import com.google.common.collect.testing.testers.CollectionAddAllTester;
import com.google.common.collect.testing.testers.CollectionAddTester;
import com.google.common.collect.testing.testers.SortedSetNavigationTester;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import junit.framework.TestSuite;

/**
Expand Down Expand Up @@ -87,13 +92,20 @@ final TestSuite createSubsetSuite(
(TestSortedSetGenerator<E>) parentBuilder.getSubjectGenerator().getInnerGenerator();

List<Feature<?>> features = new ArrayList<>(parentBuilder.getFeatures());
features.remove(CollectionFeature.ALLOWS_NULL_VALUES);
Set<Method> suppressing = new HashSet<>(parentBuilder.getSuppressedTests());
features.add(CollectionFeature.SUBSET_VIEW);
if (features.remove(CollectionFeature.ALLOWS_NULL_VALUES)) {
// the null value might be out of bounds, so we can't always construct a subset with nulls
features.add(CollectionFeature.ALLOWS_NULL_QUERIES);
// but add null might still be supported if it happens to be within range of the subset
suppressing.add(CollectionAddTester.getAddNullUnsupportedMethod());
suppressing.add(CollectionAddAllTester.getAddAllNullUnsupportedMethod());
}

return newBuilderUsing(delegate, to, from)
.named(parentBuilder.getName() + " subSet " + from + "-" + to)
.withFeatures(features)
.suppressing(parentBuilder.getSuppressedTests())
.suppressing(suppressing)
.withSetUp(parentBuilder.getSetUp())
.withTearDown(parentBuilder.getTearDown())
.createTestSuite();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.GwtCompatible;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
Expand All @@ -38,6 +40,7 @@ public static class ImmutableBiMapGenerator extends TestStringBiMapGenerator {
protected BiMap<String, String> create(Entry<String, String>[] entries) {
ImmutableBiMap.Builder<String, String> builder = ImmutableBiMap.builder();
for (Entry<String, String> entry : entries) {
checkNotNull(entry);
builder.put(entry.getKey(), entry.getValue());
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.GwtCompatible;
import com.google.common.collect.BiMap;
import com.google.common.collect.testing.DerivedGenerator;
Expand Down Expand Up @@ -112,6 +114,7 @@ public SampleElements<Entry<V, K>> samples() {
}

private Entry<V, K> reverse(Entry<K, V> entry) {
checkNotNull(entry);
return Helpers.mapEntry(entry.getValue(), entry.getKey());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ Set<Feature<?>> computeMultimapGetFeatures(Set<Feature<?>> multimapFeatures) {
if (derivedFeatures.contains(CollectionFeature.SUPPORTS_ADD)) {
derivedFeatures.add(ListFeature.SUPPORTS_ADD_WITH_INDEX);
}
if (derivedFeatures.contains(CollectionFeature.SUPPORTS_REMOVE)) {
derivedFeatures.add(ListFeature.SUPPORTS_REMOVE_WITH_INDEX);
}
if (derivedFeatures.contains(CollectionFeature.GENERAL_PURPOSE)) {
derivedFeatures.add(ListFeature.GENERAL_PURPOSE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.common.collect.testing.google;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.testing.Helpers.mapEntry;

import com.google.common.annotations.GwtCompatible;
Expand Down Expand Up @@ -53,6 +54,7 @@ public static class ImmutableMapGenerator extends TestStringMapGenerator {
protected Map<String, String> create(Entry<String, String>[] entries) {
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
for (Entry<String, String> entry : entries) {
checkNotNull(entry);
builder.put(entry.getKey(), entry.getValue());
}
return builder.buildOrThrow();
Expand Down Expand Up @@ -142,7 +144,7 @@ public List<Entry<String, Integer>> create(Object... elements) {
ImmutableMap.Builder<String, Integer> builder = ImmutableMap.builder();
for (Object o : elements) {
@SuppressWarnings("unchecked")
Entry<String, Integer> entry = (Entry<String, Integer>) o;
Entry<String, Integer> entry = (Entry<String, Integer>) checkNotNull(o);
builder.put(entry);
}
return builder.buildOrThrow().entrySet().asList();
Expand All @@ -154,7 +156,7 @@ public static class ImmutableEnumMapGenerator extends TestEnumMapGenerator {
protected Map<AnEnum, String> create(Entry<AnEnum, String>[] entries) {
Map<AnEnum, String> map = Maps.newHashMap();
for (Entry<AnEnum, String> entry : entries) {
// checkArgument(!map.containsKey(entry.getKey()));
checkNotNull(entry);
map.put(entry.getKey(), entry.getValue());
}
return Maps.immutableEnumMap(map);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void testPutAllOnPresentNullKey() {
assertGet(null, v3(), v4());
}

@MapFeature.Require(absent = ALLOWS_NULL_KEYS)
@MapFeature.Require(value = SUPPORTS_PUT, absent = ALLOWS_NULL_KEYS)
public void testPutAllNullForbidden() {
try {
multimap().putAll(null, Collections.singletonList(v3()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public List<Entry<String, Integer>> create(Object... elements) {
ImmutableSortedMap.Builder<String, Integer> builder = ImmutableSortedMap.naturalOrder();
for (Object o : elements) {
@SuppressWarnings("unchecked")
Entry<String, Integer> entry = (Entry<String, Integer>) o;
Entry<String, Integer> entry = (Entry<String, Integer>) checkNotNull(o);
builder.put(entry);
}
return builder.build().entrySet().asList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static Test suite() {
CollectionSize.ANY,
CollectionFeature.KNOWN_ORDER,
CollectionFeature.SERIALIZABLE,
CollectionFeature.ALLOWS_NULL_QUERIES)
CollectionFeature.ALLOWS_NULL_VALUES)
.named("Multisets.unmodifiableMultiset[LinkedHashMultiset]")
.createTestSuite());

Expand Down

0 comments on commit 5da71a7

Please sign in to comment.