-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add List-based support to Arguments API #4574
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
base: main
Are you sure you want to change the base?
Changes from all commits
3f3aa29
d566f95
b73ae11
87ca6b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -13,6 +13,10 @@ | |||||||||||||||||||||
import static org.apiguardian.api.API.Status.EXPERIMENTAL; | ||||||||||||||||||||||
import static org.apiguardian.api.API.Status.STABLE; | ||||||||||||||||||||||
|
||||||||||||||||||||||
import java.util.ArrayList; | ||||||||||||||||||||||
import java.util.Arrays; | ||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||
|
||||||||||||||||||||||
import org.apiguardian.api.API; | ||||||||||||||||||||||
import org.jspecify.annotations.Nullable; | ||||||||||||||||||||||
import org.junit.platform.commons.util.Preconditions; | ||||||||||||||||||||||
|
@@ -125,6 +129,80 @@ static ArgumentSet argumentSet(String name, @Nullable Object... arguments) { | |||||||||||||||||||||
return new ArgumentSet(name, arguments); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Factory method for creating an instance of {@code Arguments} based on | ||||||||||||||||||||||
* the supplied {@link List} of {@code arguments}. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @param arguments the arguments to be used for an invocation of the test | ||||||||||||||||||||||
* method; must not be {@code null} but may contain {@code null}. | ||||||||||||||||||||||
* @return an instance of {@code Arguments}; never {@code null}. | ||||||||||||||||||||||
* @since 6.0 | ||||||||||||||||||||||
Comment on lines
+136
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
* @see #argumentsFrom(List) | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
@API(status = EXPERIMENTAL, since = "6.0") | ||||||||||||||||||||||
static Arguments from(List<@Nullable Object> arguments) { | ||||||||||||||||||||||
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this use generic variance?
Suggested change
|
||||||||||||||||||||||
Preconditions.notNull(arguments, "arguments must not be null"); | ||||||||||||||||||||||
return of(arguments.toArray()); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Factory method for creating an instance of {@code Arguments} based on | ||||||||||||||||||||||
* the supplied {@link List} of {@code arguments}. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* <p>This method is an <em>alias</em> for {@link Arguments#from} and is | ||||||||||||||||||||||
* intended to be used when statically imported — for example, via: | ||||||||||||||||||||||
* {@code import static org.junit.jupiter.params.provider.Arguments.argumentsFrom;} | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @param arguments the arguments to be used for an invocation of the test | ||||||||||||||||||||||
* method; must not be {@code null} but may contain {@code null}. | ||||||||||||||||||||||
* @return an instance of {@code Arguments}; never {@code null}. | ||||||||||||||||||||||
Comment on lines
+156
to
+158
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
* @since 6.0 | ||||||||||||||||||||||
* @see #argumentSet(String, Object...) | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
@API(status = EXPERIMENTAL, since = "6.0") | ||||||||||||||||||||||
static Arguments argumentsFrom(List<@Nullable Object> arguments) { | ||||||||||||||||||||||
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
return from(arguments); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Factory method for creating an {@link ArgumentSet} based on the supplied | ||||||||||||||||||||||
* {@code name} and {@link List} of {@code arguments}. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* <p>This method is a convenient alternative to | ||||||||||||||||||||||
* {@link #argumentSet(String, Object...)} when working with {@link List} | ||||||||||||||||||||||
* based inputs. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @param name the name of the argument set; must not be {@code null} | ||||||||||||||||||||||
* or blank. | ||||||||||||||||||||||
* @param arguments the arguments to be used for an invocation of the test | ||||||||||||||||||||||
* method; must not be {@code null} but may contain {@code null}. | ||||||||||||||||||||||
* @return an {@code ArgumentSet}; never {@code null}. | ||||||||||||||||||||||
Comment on lines
+175
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
* @since 6.0 | ||||||||||||||||||||||
* @see #argumentSet(String, Object...) | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
@API(status = EXPERIMENTAL, since = "6.0") | ||||||||||||||||||||||
static ArgumentSet argumentSetFrom(String name, List<@Nullable Object> arguments) { | ||||||||||||||||||||||
Preconditions.notBlank(name, "name must not be null or blank"); | ||||||||||||||||||||||
Preconditions.notNull(arguments, "arguments list must not be null"); | ||||||||||||||||||||||
return new ArgumentSet(name, arguments.toArray()); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Convert the arguments to a new mutable {@link List} containing the same | ||||||||||||||||||||||
* elements as {@link #get()}. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* <p>This is useful for test logic that benefits from {@code List} | ||||||||||||||||||||||
* operations such as filtering, transformation, or assertions. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @return a mutable List of arguments; never {@code null} but may contain | ||||||||||||||||||||||
* {@code null}. | ||||||||||||||||||||||
Comment on lines
+197
to
+198
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
* @since 6.0 | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
@API(status = EXPERIMENTAL, since = "6.0") | ||||||||||||||||||||||
default List<@Nullable Object> toList() { | ||||||||||||||||||||||
return new ArrayList<>(Arrays.asList(get())); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Specialization of {@link Arguments} that associates a {@link #getName() name} | ||||||||||||||||||||||
* with a set of {@link #get() arguments}. | ||||||||||||||||||||||
|
@@ -171,5 +249,4 @@ public String toString() { | |||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,12 @@ | |
import static org.junit.jupiter.params.provider.Arguments.arguments; | ||
import static org.junit.jupiter.params.provider.Arguments.of; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.params.provider.Arguments.ArgumentSet; | ||
|
||
/** | ||
* Unit tests for {@link Arguments}. | ||
|
@@ -56,4 +61,75 @@ void argumentsReturnsSameArrayUsedForCreating() { | |
assertThat(arguments.get()).isSameAs(input); | ||
} | ||
|
||
@Test | ||
void fromSupportsList() { | ||
List<Object> input = Arrays.asList(1, "two", null, 3.0); | ||
Arguments arguments = Arguments.from(input); | ||
|
||
assertArrayEquals(new Object[] { 1, "two", null, 3.0 }, arguments.get()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add an assertion verifying that modifying the initial |
||
} | ||
|
||
@Test | ||
void fromSupportsListDefensiveCopy() { | ||
List<Object> input = new ArrayList<>(Arrays.asList(1, "two", null, 3.0)); | ||
Arguments arguments = Arguments.from(input); | ||
|
||
// Modify input | ||
input.set(1, "changed"); | ||
input.add("new"); | ||
|
||
// Assert that arguments are unchanged | ||
assertArrayEquals(new Object[] { 1, "two", null, 3.0 }, arguments.get()); | ||
} | ||
|
||
@Test | ||
void argumentsFromSupportsList() { | ||
List<Object> input = Arrays.asList("a", 2, null); | ||
Arguments arguments = Arguments.argumentsFrom(input); | ||
|
||
assertArrayEquals(new Object[] { "a", 2, null }, arguments.get()); | ||
} | ||
|
||
@Test | ||
void argumentSetSupportsList() { | ||
List<Object> input = Arrays.asList("x", null, 42); | ||
ArgumentSet argumentSet = Arguments.argumentSetFrom("list-test", input); | ||
|
||
assertArrayEquals(new Object[] { "x", null, 42 }, argumentSet.get()); | ||
assertThat(argumentSet.getName()).isEqualTo("list-test"); | ||
} | ||
|
||
@Test | ||
void toListReturnsMutableListOfArguments() { | ||
Arguments arguments = Arguments.of("a", 2, null); | ||
|
||
List<Object> result = arguments.toList(); | ||
|
||
assertThat(result).containsExactly("a", 2, null); // preserves content | ||
result.add("extra"); // confirms mutability | ||
assertThat(result).contains("extra"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also verify that the array in |
||
} | ||
|
||
@Test | ||
void toListDoesNotAffectInternalArgumentsState() { | ||
Arguments arguments = Arguments.of("a", 2, null); | ||
|
||
List<Object> result = arguments.toList(); | ||
result.add("extra"); // mutate the returned list | ||
|
||
// Confirm that internal state was not modified | ||
List<Object> freshCopy = arguments.toList(); | ||
assertThat(freshCopy).containsExactly("a", 2, null); | ||
} | ||
|
||
@Test | ||
void toListWorksOnEmptyArguments() { | ||
Arguments arguments = Arguments.of(); | ||
|
||
List<Object> result = arguments.toList(); | ||
|
||
assertThat(result).isEmpty(); | ||
result.add("extra"); | ||
assertThat(result).containsExactly("extra"); | ||
} | ||
} |
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.
Is It really worth documenting
never {@code null}.
? I'm afraid it adds excessive verbosity.I think the best practice is
non-null
by default, so WDYT of documenting only the places where nulls can appear rather than trying to document every possible case?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.
It may be excessive but clear. Moreover, we'd have to change that throughout the entire code base.