Skip to content

Commit

Permalink
Add entry to 1.9 changelog: Test names are no longer dependent on the…
Browse files Browse the repository at this point in the history 
… locale

Fixes #25
  • Loading branch information
@nymanjens
nymanjens committed Oct 17, 2022
1 parent bac0007 commit 110c778
Show file tree
Hide file tree
Showing 18 changed files with 797 additions and 752 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
@@ -1,6 +1,8 @@
## 1.9

- Bugfix: Support explicit ordering by the JUnit4 `@Rule`. For example: `@Rule(ordering=3)`.
- Potential test name change: Test names are no longer dependent on the locale of the machine
running it (e.g. doubles with integer values are always formatted with a trailing `.0`)

## 1.8

Expand Down
15 changes: 11 additions & 4 deletions .../main/java/com/google/testing/junit/testparameterinjector/BaseTestParameterValidator.java
View file
Expand Up @@ -18,8 +18,8 @@
import static com.google.common.base.Preconditions.checkState;
import static java.lang.Math.min;

import com.google.common.collect.FluentIterable;
import java.lang.annotation.Annotation;
import java.util.Comparator;
import java.util.List;

/**
Expand All @@ -45,10 +45,17 @@ public boolean shouldSkip(Context context) {
// so that we can easily determine which parameter value should be used for validating the
// other parameters (e.g. should this test be for (a1, b1, c1), (a2, b2, c2), or
// (a2, b2, c3). The test parameter 'C' will be the 'leadingParameter'.

Class<? extends Annotation> leadingParameter =
parameters.stream()
.max(Comparator.comparing(parameter -> context.getSpecifiedValues(parameter).size()))
.get();
FluentIterable.from(parameters)
.toSortedList(
(o1, o2) ->
Integer.compare(
context.getSpecifiedValues(o1).size(),
context.getSpecifiedValues(o2).size()))
.reverse()
.get(0);

// Second, determine which index is the current value in the specified value list of
// the leading parameter. In the example above, the index of the current value 'c2' of the
// leading parameter 'C' would be '1', given the specified values (c1, c2, c3).
Expand Down
48 changes: 25 additions & 23 deletions ...4/src/main/java/com/google/testing/junit/testparameterinjector/ParameterValueParsing.java
View file
Expand Up @@ -17,20 +17,20 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;

import com.google.common.base.Function;
import com.google.common.collect.Lists;
import com.google.common.primitives.Primitives;
import com.google.common.reflect.TypeToken;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.protobuf.ByteString;
import com.google.protobuf.MessageLite;
import java.lang.reflect.ParameterizedType;
import java.nio.charset.StandardCharsets;
import java.nio.charset.Charset;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.Map.Entry;
import javax.annotation.Nullable;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.constructor.SafeConstructor;
Expand Down Expand Up @@ -64,7 +64,7 @@ static Object parseYamlStringToObject(String yamlString) {
return new Yaml(new SafeConstructor()).load(yamlString);
}

@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked"})
static Object parseYamlObjectToJavaType(Object parsedYaml, TypeToken<?> javaType) {
// Pass along null so we don't have to worry about it below
if (parsedYaml == null) {
Expand All @@ -76,33 +76,33 @@ static Object parseYamlObjectToJavaType(Object parsedYaml, TypeToken<?> javaType

yamlValueTransformer
.ifJavaType(String.class)
.supportParsedType(String.class, identity())
.supportParsedType(String.class, self -> self)
// Also support other primitives because it's easy to accidentally write e.g. a number when
// a string was intended in YAML
.supportParsedType(Boolean.class, Object::toString)
.supportParsedType(Integer.class, Object::toString)
.supportParsedType(Long.class, Object::toString)
.supportParsedType(Double.class, Object::toString);

yamlValueTransformer.ifJavaType(Boolean.class).supportParsedType(Boolean.class, identity());
yamlValueTransformer.ifJavaType(Boolean.class).supportParsedType(Boolean.class, self -> self);

yamlValueTransformer.ifJavaType(Integer.class).supportParsedType(Integer.class, identity());
yamlValueTransformer.ifJavaType(Integer.class).supportParsedType(Integer.class, self -> self);

yamlValueTransformer
.ifJavaType(Long.class)
.supportParsedType(Long.class, identity())
.supportParsedType(Long.class, self -> self)
.supportParsedType(Integer.class, Integer::longValue);

yamlValueTransformer
.ifJavaType(Float.class)
.supportParsedType(Float.class, identity())
.supportParsedType(Float.class, self -> self)
.supportParsedType(Double.class, Double::floatValue)
.supportParsedType(Integer.class, Integer::floatValue)
.supportParsedType(String.class, Float::valueOf);

yamlValueTransformer
.ifJavaType(Double.class)
.supportParsedType(Double.class, identity())
.supportParsedType(Double.class, self -> self)
.supportParsedType(Integer.class, Integer::doubleValue)
.supportParsedType(Long.class, Long::doubleValue)
.supportParsedType(String.class, Double::valueOf);
Expand All @@ -123,8 +123,11 @@ static Object parseYamlObjectToJavaType(Object parsedYaml, TypeToken<?> javaType

yamlValueTransformer
.ifJavaType(byte[].class)
.supportParsedType(byte[].class, identity())
.supportParsedType(String.class, s -> s.getBytes(StandardCharsets.UTF_8));
.supportParsedType(byte[].class, self -> self)
// Uses String based charset because StandardCharsets was not introduced until later
// versions of Android
// See https://developer.android.com/reference/java/nio/charset/StandardCharsets.
.supportParsedType(String.class, s -> s.getBytes(Charset.forName("UTF-8")));

yamlValueTransformer
.ifJavaType(ByteString.class)
Expand All @@ -150,16 +153,15 @@ static Object parseYamlObjectToJavaType(Object parsedYaml, TypeToken<?> javaType
}

private static Map<?, ?> parseYamlMapToJavaMap(Map<?, ?> map, TypeToken<?> javaType) {
return map.entrySet().stream()
.collect(
toMap(
entry ->
parseYamlObjectToJavaType(
entry.getKey(), getGenericParameterType(javaType, /* parameterIndex= */ 0)),
entry ->
parseYamlObjectToJavaType(
entry.getValue(),
getGenericParameterType(javaType, /* parameterIndex= */ 1))));
Map<Object, Object> returnedMap = new LinkedHashMap<>();
for (Entry<?, ?> entry : map.entrySet()) {
returnedMap.put(
parseYamlObjectToJavaType(
entry.getKey(), getGenericParameterType(javaType, /* parameterIndex= */ 0)),
parseYamlObjectToJavaType(
entry.getValue(), getGenericParameterType(javaType, /* parameterIndex= */ 1)));
}
return returnedMap;
}

private static TypeToken<?> getGenericParameterType(TypeToken<?> typeToken, int parameterIndex) {
Expand Down
76 changes: 37 additions & 39 deletions junit4/src/main/java/com/google/testing/junit/testparameterinjector/PluggableTestRunner.java
View file
Expand Up @@ -14,21 +14,20 @@

package com.google.testing.junit.testparameterinjector;

import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.joining;

import com.google.common.base.Joiner;
import com.google.common.base.Throwables;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.Lists;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.Rule;
import org.junit.Test;
import org.junit.internal.runners.model.ReflectiveCallable;
Expand Down Expand Up @@ -96,14 +95,17 @@ protected boolean shouldSortTestMethodsDeterministically() {
* <p>This should be deterministic. The order should not change, even when tests are added/removed
* or between releases.
*/
protected Stream<FrameworkMethod> sortTestMethods(Stream<FrameworkMethod> methods) {
protected ImmutableList<FrameworkMethod> sortTestMethods(ImmutableList<FrameworkMethod> methods) {
if (!shouldSortTestMethodsDeterministically()) {
return methods;
}

return methods.sorted(
comparing((FrameworkMethod method) -> method.getName().hashCode())
.thenComparing(FrameworkMethod::getName));
return FluentIterable.from(methods)
.toSortedList(
(o1, o2) ->
ComparisonChain.start()
.compare(o1.getName().hashCode(), o2.getName().hashCode())
.compare(o1.getName(), o2.getName())
.result());
}

/**
Expand All @@ -125,14 +127,11 @@ protected List<TestRule> getExtraTestRules() {

@Override
protected final ImmutableList<FrameworkMethod> computeTestMethods() {
Stream<FrameworkMethod> processedMethods =
getSupportedTestAnnotations().stream()
.flatMap(annotation -> getTestClass().getAnnotatedMethods(annotation).stream())
.flatMap(method -> processMethod(method).stream());

processedMethods = sortTestMethods(processedMethods);

return processedMethods.collect(toImmutableList());
return sortTestMethods(
FluentIterable.from(getSupportedTestAnnotations())
.transformAndConcat(annotation -> getTestClass().getAnnotatedMethods(annotation))
.transformAndConcat(this::processMethod)
.toList());
}

/** Implementation of a JUnit FrameworkMethod where the name and annotation list is overridden. */
Expand Down Expand Up @@ -182,11 +181,13 @@ public int hashCode() {
}

private ImmutableList<FrameworkMethod> processMethod(FrameworkMethod initialMethod) {
return getTestMethodProcessors()
.calculateTestInfos(initialMethod.getMethod(), getTestClass().getJavaClass())
.stream()
.map(testInfo -> new OverriddenFrameworkMethod(testInfo.getMethod(), testInfo))
.collect(toImmutableList());
return FluentIterable.from(
getTestMethodProcessors()
.calculateTestInfos(initialMethod.getMethod(), getTestClass().getJavaClass()))
.transform(
testInfo ->
(FrameworkMethod) new OverriddenFrameworkMethod(testInfo.getMethod(), testInfo))
.toList();
}

// Note: This is a copy of the parent implementation, except that instead of calling
Expand Down Expand Up @@ -257,15 +258,16 @@ private Statement withRules(FrameworkMethod method, Object target, Statement sta
testClass.collectAnnotatedMethodValues(target, Rule.class, TestRule.class, collector::accept);
testClass.collectAnnotatedFieldValues(target, Rule.class, TestRule.class, collector::accept);

ArrayList<Integer> keys = new ArrayList<>(orderToRulesMultimap.keySet());
Collections.sort(keys);
ImmutableList<Object> orderedRules =
orderToRulesMultimap.keySet().stream()
.sorted()
.flatMap(
FluentIterable.from(keys)
.transformAndConcat(
// Execute the rules in the reverse order of when the fields occurred. This may look
// counter-intuitive, but that is what the default JUnit4 runner does, and there is
// no reason to deviate from that here.
key -> Lists.reverse(orderToRulesMultimap.get(key)).stream())
.collect(toImmutableList());
key -> Lists.reverse(orderToRulesMultimap.get(key)))
.toList();

// Note: The perceived order* is the reverse of the order in which the code below applies the
// rules to the statements because each subsequent rule wraps the previous statement.
Expand Down Expand Up @@ -334,9 +336,9 @@ protected final void validateZeroArgConstructor(List<Throwable> errorsReturned)
@Override
protected final void validateTestMethods(List<Throwable> errorsReturned) {
List<FrameworkMethod> testMethods =
getSupportedTestAnnotations().stream()
.flatMap(annotation -> getTestClass().getAnnotatedMethods(annotation).stream())
.collect(toImmutableList());
FluentIterable.from(getSupportedTestAnnotations())
.transformAndConcat(annotation -> getTestClass().getAnnotatedMethods(annotation))
.toList();
for (FrameworkMethod testMethod : testMethods) {
ExecutableValidationResult validationResult =
getTestMethodProcessors()
Expand Down Expand Up @@ -371,9 +373,9 @@ protected final void collectInitializationErrors(List<Throwable> errors) {
String.format(
"Found %s issues while initializing the test runner:\n\n - %s\n\n\n",
errors.size(),
errors.stream()
.map(Throwables::getStackTraceAsString)
.collect(joining("\n\n\n - "))));
FluentIterable.from(errors)
.transform(Throwables::getStackTraceAsString)
.join(Joiner.on("\n\n\n - "))));
}
}

Expand Down Expand Up @@ -414,8 +416,4 @@ public void evaluate() throws Throwable {
};
}
}

private static <E> Collector<E, ?, ImmutableList<E>> toImmutableList() {
return Collectors.collectingAndThen(Collectors.toList(), ImmutableList::copyOf);
}
}

0 comments on commit 110c778

Please sign in to comment.