-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Introduce named argument set support for @ParameterizedTest
#3818
Comments
Team decision: Spike the following: return Stream.of(
namedArguments("Test Case Name", firstArg, secondArg, thirdArg),
namedArguments("Other Case Name", arguments...)
); where |
NamedArguments
to name a set of arguments in a @ParameterizedTest
@marcphilipp @sbrannen if I understood it correctly, the usage of |
@scordio, I've already spiked this locally and will push to a feature branch soon.
That's correct: the goal is that it will work seamlessly, and it does in my current spike. However, having said that, it does appear that we'll need to introduce a new display name holder and include that in the default display name pattern. Otherwise, I don't yet see how to reliably include this additional information. Though maybe we can work around that by introducing an internal placeholder for this purpose. Time will tell... It's a work in progress. 😉 |
…edTest This is a proof of concept which demonstrates one way to achieve this. Running NamedArgumentsSetDemo, results in the following output: Important Files :: [1] file1=VIP, file2=path2 [2] file1=path3, file2=path4 If we decide to implement this feature, we will need to revisit the current implementation of ParameterizedTestNameFormatter and consider introducing a new placeholder for display names for parameterized tests in order to allow users to specify inclusion of the "name of a set of arguments". See junit-team#3818
Very cool, looking forward to it! |
@ParameterizedTest This is a second variant of how this could be implemented, which revises the implementation in the previous commit so that running NamedArgumentsSetDemo, now results in the following output: Important Files Other Files With the approach in this commit, if arguments are supplied as NamedArguments, the display name pattern is completely ignored, and the display name for the current parameterized invocation is set to the value returned from NamedArguments#getName. See junit-team#3818
Current work on this issue can be viewed in the following feature branch. main...sbrannen:junit5:issues/3818-NamedArguments I implemented two approaches that we can consider and use for further brainstorming. The first commit prepends the value returned This results in display names for two invocations like this (where "VIP" is from the use of
Though, in the description of this issue @aws-gibbskt suggested that the "group name" replace what would otherwise be the display name. The second commit takes that in account and results in display names for two invocations like this (where both invocations use
Feedback and further brainstorming is welcome! |
In summary, the aforementioned feature branch currently supports a test class like this: // imports ...
import static org.junit.jupiter.params.provider.Arguments.namedArguments;
class NamedArgumentsSetDemo {
@ParameterizedTest
@FieldSource
void namedArgumentsSet(File file1, File file2, TestInfo testInfo) {
System.out.println(testInfo.getDisplayName());
}
static List<Arguments> namedArgumentsSet = Arrays.asList(
namedArguments("Important Files", new File("path1"), new File("path2")),
namedArguments("Other Files", new File("path3"), new File("path4"))
);
} Which outputs the following for the display names:
|
I think including the index (by default) would be helpful:
|
I agree. I'll make that change. Though we need to decide if we want this to actually be configurable or hard-coded to do exactly that and only that. Thoughts? One compelling reason not to make it configurable is that the use of Along that line of thinking, I imagined that |
…edTest This is a third variant of how this could be implemented, which revises the implementation in the previous commit so that the invocation index is always included when NamedArguments is used, resulting in the following output for the NamedArgumentsSetDemo: [1] Important Files [2] Other Files See junit-team#3818
The third commit always includes the invocation index and results in display names for two invocations like this (where both invocations use
|
This looks like a great addition to junit and addresses the intent of my request.
or
as the default behavior are both fine. |
Rationale: there is no benefit to implementing Named<Arguments>, since we never need access to the "payload". The NamedArguments on its own is sufficient for this use case. See junit-team#3818
Running NamedArgumentsSetDemo, now results in the following output for the two parameterized test methods: Default display name: [1] Important Files [2] Other Files Custom display name ("{namedArguments} :: {argumentsWithNames}"): Important Files :: file1=path1, file2=path2 Other Files :: file1=path3, file2=path4 See junit-team#3818
With the latest changes on my feature branch, the following parameterized tests... @BeforeEach
void printDisplayName(TestInfo testInfo) {
System.out.println(testInfo.getDisplayName());
}
@ParameterizedTest
@FieldSource("namedArgumentsList")
void defaultNamedArgumentsDisplayName(File file1, File file2) {
}
@ParameterizedTest(name = "{namedArguments} :: {argumentsWithNames}")
@FieldSource("namedArgumentsList")
void customNamedArgumentsDisplayName(File file1, File file2) {
}
static List<Arguments> namedArgumentsList = Arrays.asList(
namedArguments("Important Files", new File("path1"), new File("path2")),
namedArguments("Other Files", new File("path3"), new File("path4"))
); ... now output this:
The default pattern for String DEFAULT_NAMED_ARGUMENTS_DISPLAY_NAME = "[" + INDEX_PLACEHOLDER + "] " + NAMED_ARGUMENTS_PLACEHOLDER; And that new placeholder is: String NAMED_ARGUMENTS_PLACEHOLDER = "{namedArguments}"; |
The reason I posted the above is to discuss naming, because... as we all know... Naming is hard! ™️ 😉 The new type is called I think those names are probably fine as-is, but I'm not happy with the names/values of the constants/placeholders. When you see What's the difference between "named arguments" and "arguments with names"? If we go with that "technical" naming strategy, I fear it will be too confusing for users (even with good documentation). So I'd like to brainstorm about better names for those constants and the placeholder. Technically, we're talking about a "set of arguments with a name for that entire set." We are not actually talking about "named arguments". So, even if we stick with We could go very technical with something like "NamedArguments name" (because of String NAMED_ARGUMENTS_NAME_PLACEHOLDER = "{namedArgumentsName}";
String DEFAULT_NAMED_ARGUMENTS_DISPLAY_NAME = "[" + INDEX_PLACEHOLDER + "] " + NAMED_ARGUMENTS_NAME_PLACEHOLDER; But I think that's extremely ugly, and I'm only including it for the sake of argument (pun intended). Since we're talking about a "set of arguments" or an "argument set", perhaps something like the following would be better. String ARGUMENT_SET_NAME_PLACEHOLDER = "{argumentSetName}";
String DEFAULT_ARGUMENT_SET_DISPLAY_NAME = "[" + INDEX_PLACEHOLDER + "] " + ARGUMENT_SET_NAME_PLACEHOLDER; Or perhaps we should refer to this thing as an "argument group". Any suggestions or brainstorming ideas are welcome! 👍 p.s. Another idea I had was to refer to the name of this new thing as the "invocation name" since you're effectively assigning a name to the current parameterized test invocation when you use |
I'm in favor of introducing a term like "argument set", "invocation name", or "scenario" to refer to. Depending on which we choose, this might also mean renaming Why do we need |
Thanks for the feedback, @marcphilipp.
I think we should shy away from "scenario" since that might get confused with "scenario tests" which we don't support, and I basically convinced myself above that "invocation name" is probably too generic. So, I'm currently leaning toward "argument set" for the terminology and
Indeed, that would help to avoid confusion. Perhaps the static factory method should be named |
I hadn't considered that option. My goal was to make it configurable and clear within the available options/constants in I'll experiment with that. |
NamedArguments
to name a set of arguments in a @ParameterizedTest
@ParameterizedTest
Running ArgumentSetDemo, results in the following output: [1] Important Files [2] Other Files Important Files :: path1, path2 Other Files :: path3, path4 [1] Mixed Arguments Types :: Important Files [2] Mixed Arguments Types :: file1=path3, file2=path4 Closes junit-team#3818
Running ArgumentSetDemo, results in the following output: [1] Important Files [2] Other Files Important Files :: path1, path2 Other Files :: path3, path4 [1] Mixed Arguments Types :: Important Files [2] Mixed Arguments Types :: file1=path3, file2=path4 Closes junit-team#3818
I have finalized my proposal and submitted PR #3825, which is ready for review. 😎 |
Running ArgumentSetDemo, results in the following output: [1] Important Files [2] Other Files Important Files :: path1, path2 Other Files :: path3, path4 [1] Mixed Arguments Types :: Important Files [2] Mixed Arguments Types :: file1=path3, file2=path4 Closes junit-team#3818
Prior to this commit, it was possible to use the Named API to provide custom names for individual arguments for a @ParameterizedTest when using @MethodSource, @FieldSource, or @ArgumentsSource, but it was not possible to provide a custom name for an entire set of arguments without resolving to obtuse workarounds. To address that, this commit introduces an argumentSet(String,Object...) static factory method in Arguments for providing a custom name for an entire set of arguments for a @ParameterizedTest. This commit also introduces the following placeholders for the display name of a @ParameterizedTest. - {argumentSetName} :: the name of the argument set - {argumentSetNameOrArgumentsWithNames} :: resolves to {argumentSetName} or {argumentsWithNames}, depending on how the arguments are supplied In addition, the default display name for a @ParameterizedTest is now "[{index}] {argumentSetNameOrArgumentsWithNames}". For example, given the following... @ParameterizedTest @FieldSource void argumentSets(File file1, File file2) { } static List<Arguments> argumentSets = List.of( argumentSet("Important files", new File("path1"), new File("path2")), argumentSet("Other files", new File("path3"), new File("path4")) ); ... the resulting display names are: [1] Important files [2] Other files Closes junit-team#3818
Prior to this commit, it was possible to use the Named API to provide custom names for individual arguments for a @ParameterizedTest when using @MethodSource, @FieldSource, or @ArgumentsSource, but it was not possible to provide a custom name for an entire set of arguments without resolving to obtuse workarounds. To address that, this commit introduces an argumentSet(String,Object...) static factory method in Arguments for providing a custom name for an entire set of arguments for a @ParameterizedTest. This commit also introduces the following placeholders for the display name of a @ParameterizedTest. - {argumentSetName} :: the name of the argument set - {argumentSetNameOrArgumentsWithNames} :: resolves to {argumentSetName} or {argumentsWithNames}, depending on how the arguments are supplied In addition, the default display name for a @ParameterizedTest is now "[{index}] {argumentSetNameOrArgumentsWithNames}". For example, given the following... @ParameterizedTest @FieldSource void argumentSets(File file1, File file2) { } static List<Arguments> argumentSets = List.of( argumentSet("Important files", new File("path1"), new File("path2")), argumentSet("Other files", new File("path3"), new File("path4")) ); ... the resulting display names are: [1] Important files [2] Other files Closes junit-team#3818
Prior to this commit, it was possible to use the Named API to provide custom names for individual arguments for a @ParameterizedTest when using @MethodSource, @FieldSource, or @ArgumentsSource, but it was not possible to provide a custom name for an entire set of arguments without resolving to obtuse workarounds. To address that, this commit introduces an argumentSet(String,Object...) static factory method in Arguments for providing a custom name for an entire set of arguments for a @ParameterizedTest. This commit also introduces the following placeholders for the display name of a @ParameterizedTest. - {argumentSetName} :: the name of the argument set - {argumentSetNameOrArgumentsWithNames} :: resolves to {argumentSetName} or {argumentsWithNames}, depending on how the arguments are supplied In addition, the default display name for a @ParameterizedTest is now "[{index}] {argumentSetNameOrArgumentsWithNames}". For example, given the following... @ParameterizedTest @FieldSource void argumentSets(File file1, File file2) { } static List<Arguments> argumentSets = List.of( argumentSet("Important files", new File("path1"), new File("path2")), argumentSet("Other files", new File("path3"), new File("path4")) ); ... the resulting display names are: [1] Important files [2] Other files See #3825 Closes #3818
Enjoy! 😎 |
Summary
I find myself writing the following.
#2301 introduced a way to name an argument, but it feels very unnatural to be used for multiple argument test cases.
The recommendation in that issue is to do the below with the
name
as defined above.I would much rather have something like:
Related Issues
Proposal
Provide the ability to name an individual parameterized test invocation by wrapping the
Arguments
inNamed<Arguments>
rather than wrapping a specific argument inNamed<T>
.The text was updated successfully, but these errors were encountered: