-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Use Guava testlib for collection tests #2746
Merged
eamonnmcmanus
merged 2 commits into
google:main
from
Marcono1234:marcono1234/guava-collection-testlib
Oct 9, 2024
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
68 changes: 68 additions & 0 deletions
68
gson/src/test/java/com/google/gson/JsonArrayAsListSuiteTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
package com.google.gson; | ||
|
||
import com.google.common.collect.testing.ListTestSuiteBuilder; | ||
import com.google.common.collect.testing.SampleElements; | ||
import com.google.common.collect.testing.TestListGenerator; | ||
import com.google.common.collect.testing.features.CollectionFeature; | ||
import com.google.common.collect.testing.features.CollectionSize; | ||
import com.google.common.collect.testing.features.ListFeature; | ||
import java.util.List; | ||
import junit.framework.Test; | ||
import org.junit.runner.RunWith; | ||
import org.junit.runners.AllTests; | ||
|
||
/** | ||
* Dynamic {@link ListTestSuiteBuilder List test suite} for {@link JsonArray#asList()}. This | ||
* complements {@link JsonArrayAsListTest}, which can cover some cases which are not covered here, | ||
* e.g. making sure changes in the {@code List} view are visible in the {@code JsonArray}. | ||
*/ | ||
@RunWith(AllTests.class) | ||
public class JsonArrayAsListSuiteTest { | ||
private static class ListGenerator implements TestListGenerator<JsonElement> { | ||
@Override | ||
public SampleElements<JsonElement> samples() { | ||
return new SampleElements<JsonElement>( | ||
JsonNull.INSTANCE, | ||
new JsonPrimitive(true), | ||
new JsonPrimitive("test"), | ||
new JsonArray(), | ||
new JsonObject()); | ||
} | ||
|
||
@Override | ||
public JsonElement[] createArray(int length) { | ||
return new JsonElement[length]; | ||
} | ||
|
||
@Override | ||
public Iterable<JsonElement> order(List<JsonElement> insertionOrder) { | ||
return insertionOrder; | ||
} | ||
|
||
@Override | ||
public List<JsonElement> create(Object... elements) { | ||
JsonArray array = new JsonArray(); | ||
for (Object element : elements) { | ||
array.add((JsonElement) element); | ||
} | ||
return array.asList(); | ||
} | ||
} | ||
|
||
// Special method recognized by JUnit's `AllTests` runner | ||
public static Test suite() { | ||
return ListTestSuiteBuilder.using(new ListGenerator()) | ||
.withFeatures( | ||
CollectionSize.ANY, | ||
// Note: There is current a Guava bug which causes 'null additions' to not be tested if | ||
// 'null queries' is enabled, see https://github.com/google/guava/issues/7401 | ||
CollectionFeature.ALLOWS_NULL_QUERIES, | ||
CollectionFeature.RESTRICTS_ELEMENTS, // List only allows JsonElement | ||
CollectionFeature.SUPPORTS_ADD, | ||
ListFeature.REMOVE_OPERATIONS, | ||
ListFeature.SUPPORTS_ADD_WITH_INDEX, | ||
ListFeature.SUPPORTS_SET) | ||
.named("JsonArray#asList") | ||
.createTestSuite(); | ||
} | ||
} |
86 changes: 86 additions & 0 deletions
86
gson/src/test/java/com/google/gson/JsonObjectAsMapSuiteTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package com.google.gson; | ||
|
||
import com.google.common.collect.testing.MapTestSuiteBuilder; | ||
import com.google.common.collect.testing.SampleElements; | ||
import com.google.common.collect.testing.TestMapGenerator; | ||
import com.google.common.collect.testing.features.CollectionFeature; | ||
import com.google.common.collect.testing.features.CollectionSize; | ||
import com.google.common.collect.testing.features.MapFeature; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
import junit.framework.Test; | ||
import org.junit.runner.RunWith; | ||
import org.junit.runners.AllTests; | ||
|
||
/** | ||
* Dynamic {@link MapTestSuiteBuilder Map test suite} for {@link JsonObject#asMap()}. This | ||
* complements {@link JsonObjectAsMapTest}, which can cover some cases which are not covered here, | ||
* e.g. making sure changes in the {@code Map} view are visible in the {@code JsonObject}. | ||
*/ | ||
@RunWith(AllTests.class) | ||
public class JsonObjectAsMapSuiteTest { | ||
private static class MapGenerator implements TestMapGenerator<String, JsonElement> { | ||
@Override | ||
public SampleElements<Entry<String, JsonElement>> samples() { | ||
return new SampleElements<>( | ||
Map.entry("one", JsonNull.INSTANCE), | ||
Map.entry("two", new JsonPrimitive(true)), | ||
Map.entry("three", new JsonPrimitive("test")), | ||
Map.entry("four", new JsonArray()), | ||
Map.entry("five", new JsonObject())); | ||
} | ||
|
||
@Override | ||
public Map<String, JsonElement> create(Object... elements) { | ||
JsonObject object = new JsonObject(); | ||
for (Object element : elements) { | ||
var entry = (Entry<?, ?>) element; | ||
object.add((String) entry.getKey(), (JsonElement) entry.getValue()); | ||
} | ||
return object.asMap(); | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
@Override | ||
public Entry<String, JsonElement>[] createArray(int length) { | ||
return (Entry<String, JsonElement>[]) new Entry<?, ?>[length]; | ||
} | ||
|
||
@Override | ||
public Iterable<Entry<String, JsonElement>> order( | ||
List<Entry<String, JsonElement>> insertionOrder) { | ||
// Preserves insertion order | ||
return insertionOrder; | ||
} | ||
|
||
@Override | ||
public String[] createKeyArray(int length) { | ||
return new String[length]; | ||
} | ||
|
||
@Override | ||
public JsonElement[] createValueArray(int length) { | ||
return new JsonElement[length]; | ||
} | ||
} | ||
|
||
// Special method recognized by JUnit's `AllTests` runner | ||
public static Test suite() { | ||
return MapTestSuiteBuilder.using(new MapGenerator()) | ||
.withFeatures( | ||
CollectionSize.ANY, | ||
// Note: There is current a Guava bug which causes 'null additions' to not be tested if | ||
// 'null queries' is enabled, see https://github.com/google/guava/issues/7401 | ||
MapFeature.ALLOWS_ANY_NULL_QUERIES, | ||
MapFeature.RESTRICTS_KEYS, // Map only allows String keys | ||
MapFeature.RESTRICTS_VALUES, // Map only allows JsonElement values | ||
MapFeature.SUPPORTS_PUT, | ||
MapFeature.SUPPORTS_REMOVE, | ||
// Affects keySet, values and entrySet (?) | ||
CollectionFeature.KNOWN_ORDER, // Map preserves insertion order | ||
CollectionFeature.SUPPORTS_ITERATOR_REMOVE) | ||
.named("JsonObject#asMap") | ||
.createTestSuite(); | ||
} | ||
} |
83 changes: 83 additions & 0 deletions
83
gson/src/test/java/com/google/gson/internal/LinkedTreeMapSuiteTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
package com.google.gson.internal; | ||
|
||
import com.google.common.collect.testing.MapTestSuiteBuilder; | ||
import com.google.common.collect.testing.TestStringMapGenerator; | ||
import com.google.common.collect.testing.features.CollectionFeature; | ||
import com.google.common.collect.testing.features.CollectionSize; | ||
import com.google.common.collect.testing.features.Feature; | ||
import com.google.common.collect.testing.features.MapFeature; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
import junit.framework.Test; | ||
import junit.framework.TestSuite; | ||
import org.junit.runner.RunWith; | ||
import org.junit.runners.AllTests; | ||
|
||
/** | ||
* Dynamic {@link MapTestSuiteBuilder Map test suite} for {@link LinkedTreeMap}. This complements | ||
* {@link LinkedTreeMapTest}. | ||
*/ | ||
@RunWith(AllTests.class) | ||
public class LinkedTreeMapSuiteTest { | ||
private static class MapGenerator extends TestStringMapGenerator { | ||
private final boolean allowNullValues; | ||
|
||
public MapGenerator(boolean allowNullValues) { | ||
this.allowNullValues = allowNullValues; | ||
} | ||
|
||
@Override | ||
protected Map<String, String> create(Entry<String, String>[] entries) { | ||
var map = new LinkedTreeMap<String, String>(allowNullValues); | ||
for (var entry : entries) { | ||
map.put(entry.getKey(), entry.getValue()); | ||
} | ||
return map; | ||
} | ||
} | ||
|
||
private static Feature<?>[] createFeatures(Feature<?>... additionalFeatures) { | ||
// Don't specify CollectionFeature.SERIALIZABLE because Guava testlib seems to assume | ||
// deserialized Map has same properties as original map (e.g. disallowing null keys), but this | ||
// is not the case for LinkedTreeMap which is serialized as JDK LinkedHashMap | ||
var features = | ||
new ArrayList<Feature<?>>( | ||
List.of( | ||
CollectionSize.ANY, | ||
// Note: There is current a Guava bug which causes 'null additions' to not be tested | ||
// if 'null queries' is enabled, see https://github.com/google/guava/issues/7401 | ||
MapFeature.ALLOWS_ANY_NULL_QUERIES, | ||
MapFeature.RESTRICTS_KEYS, // Map only allows comparable keys | ||
MapFeature.SUPPORTS_PUT, | ||
MapFeature.SUPPORTS_REMOVE, | ||
// Affects keySet, values and entrySet (?) | ||
CollectionFeature.KNOWN_ORDER, // Map preserves insertion order | ||
CollectionFeature.SUPPORTS_ITERATOR_REMOVE)); | ||
features.addAll(Arrays.asList(additionalFeatures)); | ||
return features.toArray(Feature[]::new); | ||
} | ||
|
||
// Special method recognized by JUnit's `AllTests` runner | ||
public static Test suite() { | ||
var nullValuesSuite = | ||
MapTestSuiteBuilder.using(new MapGenerator(true)) | ||
.withFeatures(createFeatures(MapFeature.ALLOWS_NULL_VALUES)) | ||
.named("nullValues=true") | ||
.createTestSuite(); | ||
|
||
var nonNullValuesSuite = | ||
MapTestSuiteBuilder.using(new MapGenerator(false)) | ||
.withFeatures(createFeatures()) | ||
.named("nullValues=false") | ||
.createTestSuite(); | ||
|
||
TestSuite testSuite = new TestSuite("LinkedTreeMap"); | ||
testSuite.addTest(nullValuesSuite); | ||
testSuite.addTest(nonNullValuesSuite); | ||
|
||
return testSuite; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We typically use the
GENERAL_PURPOSE
features when applicable. There's such a constant forList
and also forMap
. I haven't confirmed whether it applies here, and it's fine to use an explicit list instead even if it would apply, but I wanted to at least point it out.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint! I wasn't completely sure whether they were suitable because
CollectionFeature.GENERAL_PURPOSE
says:And the
List
andMap
here are not 'general-purpose' in the sense of thejava.util.Collection
definition.