Add ParameterizedTest#argumentCountValidation #4045

JonasJebing · 2024-10-04T20:38:22Z

Overview

This allows parameterized tests to fail
when there are more arguments provided than declared by the test method. This is done in a backwards compatible way
by only enabling that validation when the new
junit.jupiter.params.argumentCountValidation is set to strict or ParameterizedTest#argumentCountValidation is set to ArgumentCountValidationMode.STRICT.

Open Questions

~~Should these additions be declared as experimental or stable?~~
~~Should this feature be documented in the User Guide and Release Notes, given that it is declared as experimental?~~
Should the new precondition be documented on org.junit.jupiter.params.ParameterizedTestExtension#provideTestTemplateInvocationContexts, even though it is just an interface override and none of the existing preconditions are documented there?

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes

This allows parameterized tests to fail when there are more arguments provided than declared by the test method. This is done in a backwards compatible way by only enabling that validation when the new `junit.jupiter.params.argumentCountValidation` is set to `strict` or `ParameterizedTest#argumentCountValidation` is set to `ArgumentCountValidationMode.STRICT`.

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

marcphilipp · 2024-10-08T13:46:09Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

+					logger.warn(() -> String.format(
+						"Ignored invalid configuration '%s' set via the '%s' configuration parameter.", value, key));


The error message should be the same as in EnumConfigurationParameterConverter. Unfortunately, we can't (easily) reuse that here.

Is there some way that we could make it more easily reusable across the project? Maybe I could add another PR to address that after merging this one.
For now, I basically copied the messages from EnumConfigurationParameterConverter.

jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java

marcphilipp · 2024-10-08T13:52:59Z

Should these additions be declared as experimental or stable?

Our guideline is to introduce new features as "experimental".

Should this feature be documented in the User Guide and Release Notes, given that it is declared as experimental?

I think we should add a small sample to the User Guide otherwise the chance to get feedback is slim.

Should the new precondition be documented on org.junit.jupiter.params.ParameterizedTestExtension#provideTestTemplateInvocationContexts, even though it is just an interface override and none of the existing preconditions are documented there?

Documenting preconditions is mainly interesting for public API so we can omit it here since ParameterizedTestExtension is package-private.

# Conflicts: # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTest.java # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

- add `writing-tests-parameterized-tests-argument-count-validation` section to the user guide. - cache configuration parameter in `ExtensionContext.Store` - adjust log and error messages Issue junit-team#3708

Issue junit-team#3708

JonasJebing · 2024-10-23T14:24:51Z

Thank you for all the pointers and comments on this PR @marcphilipp. I've addressed all your comments. Could you have another look?

Issue junit-team#3708

marcphilipp

Thanks for making those changes! I think we're very close now. Please also add an entry to the 5.12.0-M1 release notes.

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ArgumentCountValidationMode.java

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTest.java

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

release-notes-5.12.0-M1-junit-jupiter-new-features-and-improvements to be exact Issue junit-team#3708

Co-authored-by: Marc Philipp <[email protected]>

documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-M1.adoc

JonasJebing · 2024-10-25T10:46:36Z

@marcphilipp could you review again? I applied all suggestions and added a release note.

documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-M1.adoc

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

Co-authored-by: Marc Philipp <[email protected]>

Issue junit-team#3708

marcphilipp

Looks like @ExpectToFail doesn't work as I had expected. 😅 I'll take a closer look.

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

This should result in invocations with valid argument counts still being executed and only invocations with invalid argument counts fail. Issue junit-team#3708

Issue junit-team#3708

JonasJebing · 2024-11-13T08:52:26Z

@marcphilipp do you know how to fix the DiskLruCache issue on the Windows check? I see some PRs, like #4120 are having the same issue.

marcphilipp · 2024-11-13T12:02:07Z

@marcphilipp do you know how to fix the DiskLruCache issue on the Windows check? I see some PRs, like #4120 are having the same issue.

Not sure. It might be a problem that started after updating Gradle to 8.11. 🤔

marcphilipp · 2024-11-13T12:10:33Z

A rerun succeeded. It does seem to happen since yesterday's Gradle update. I'll keep an eye on it.

JonasJebing · 2024-11-13T13:46:28Z

@marcphilipp could you review the latest changes? Like you suggested, the ArgumentCountValidator now only fails the invocations that have too many arguments and still runs the other invocations that have the correct number of arguments and I also added an integration test for that case.

marcphilipp

Looks good! I've only two small suggestions. Please let me know if you have time to do those or whether I should take over.

marcphilipp · 2024-11-13T16:46:27Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ArgumentCountValidator.java

+	}
+
+	private ExtensionContext.Store getStore(ExtensionContext context) {
+		return context.getRoot().getStore(ExtensionContext.Namespace.create(getClass()));


Please create a constant for ExtensionContext.Namespace.create(ArgumentCountValidator.class).

marcphilipp · 2024-11-13T16:47:39Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ArgumentCountValidator.java

+		ParameterizedTest parameterizedTest = findAnnotation(//
+			extensionContext.getRequiredTestMethod(), ParameterizedTest.class//
+		).orElseThrow(NoSuchElementException::new);


Instead of looking up the annotation, we should pass ParameterizedTestMethodContext to the constructor

JonasJebing · 2024-11-14T11:44:58Z

Looks good! I've only two small suggestions. Please let me know if you have time to do those or whether I should take over.

Thanks! I'll make those changes you suggested today or tomorrow :)

Issue junit-team#3708

JonasJebing · 2024-11-14T12:05:52Z

@marcphilipp could you merge the PR? And of course feel free to review the small change you suggested.

marcphilipp · 2024-11-17T16:16:17Z

@JonasJebing Thank you for your contribution! 👍

JonasJebing force-pushed the argument-count-validation-mode branch from 2b42de8 to 2e56d56 Compare October 5, 2024 20:30

marcphilipp requested changes Oct 8, 2024

View reviewed changes

marcphilipp linked an issue Oct 8, 2024 that may be closed by this pull request

@ParameterizedTest ignores additional arguments beyond formal parameter list #3708

Closed

JonasJebing added 3 commits October 23, 2024 12:17

Merge branch 'main' into argument-count-validation-mode

3d40ebe

# Conflicts: # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTest.java # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

address PR comments

98cdf69

- add `writing-tests-parameterized-tests-argument-count-validation` section to the user guide. - cache configuration parameter in `ExtensionContext.Store` - adjust log and error messages Issue junit-team#3708

gradle spotlessApply

89a7443

Issue junit-team#3708

JonasJebing requested a review from marcphilipp October 23, 2024 14:24

JonasJebing added 4 commits October 23, 2024 16:37

move example to .adoc because it's a test that should error

5c71632

Issue junit-team#3708

fix newline char assertion for Windows

edd9f46

Issue junit-team#3708

gradle spotlessApply

beb53ab

Issue junit-team#3708

Merge branch 'main' into argument-count-validation-mode

b117b69

JonasJebing marked this pull request as ready for review October 23, 2024 15:59

marcphilipp requested changes Oct 24, 2024

View reviewed changes

JonasJebing and others added 7 commits October 24, 2024 09:58

Merge branch 'main' into argument-count-validation-mode

37216f8

add change to release-notes 5.12.0-M1

f87404f

release-notes-5.12.0-M1-junit-jupiter-new-features-and-improvements to be exact Issue junit-team#3708

fix ArgumentCountValidationMode javadoc typo

14a071b

Co-authored-by: Marc Philipp <[email protected]>

improve ParameterizedTest javadoc

762c39a

Co-authored-by: Marc Philipp <[email protected]>

use underscores for unused lambda parameter

8a84dc2

Co-authored-by: Marc Philipp <[email protected]>

use root context store for caching config value

09ced82

Co-authored-by: Marc Philipp <[email protected]>

remove mention of experimental status from release note

0bf0b05

JonasJebing force-pushed the argument-count-validation-mode branch from 65cd018 to 0bf0b05 Compare October 24, 2024 08:43

JonasJebing commented Oct 24, 2024

View reviewed changes

documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-M1.adoc Outdated Show resolved Hide resolved

JonasJebing requested a review from marcphilipp October 24, 2024 08:57

Merge branch 'main' into argument-count-validation-mode

f189f07

marcphilipp requested changes Oct 25, 2024

View reviewed changes

@ParameterizedTests to Parameterized tests

36e7729

Co-authored-by: Marc Philipp <[email protected]>

JonasJebing and others added 2 commits October 25, 2024 15:23

improve release note

851a4db

Co-authored-by: Marc Philipp <[email protected]>

move user guide example to ParameterizedTestDemo

a986bba

Issue junit-team#3708

marcphilipp self-requested a review October 29, 2024 09:23

marcphilipp reviewed Oct 29, 2024

View reviewed changes

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java Outdated Show resolved Hide resolved

marcphilipp requested changes Oct 29, 2024

View reviewed changes

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java Outdated Show resolved Hide resolved

JonasJebing added 4 commits November 12, 2024 16:12

Merge branch 'main' into argument-count-validation-mode

2dafffb

move argument count validation to happen later

e70240c

This should result in invocations with valid argument counts still being executed and only invocations with invalid argument counts fail. Issue junit-team#3708

update javadoc to use new ArgumentCountValidator

3d84b9f

Issue junit-team#3708

retrigger checks

dd6b55a

JonasJebing requested a review from marcphilipp November 13, 2024 13:46

marcphilipp approved these changes Nov 13, 2024

View reviewed changes

JonasJebing added 2 commits November 14, 2024 12:48

Merge branch 'main' into argument-count-validation-mode

1151f6b

add small ArgumentCountValidator perf optimisations

906881c

Issue junit-team#3708

JonasJebing added 2 commits November 15, 2024 14:37

Merge branch 'main' into argument-count-validation-mode

89ceaec

Merge branch 'main' into argument-count-validation-mode

9619c36

marcphilipp approved these changes Nov 17, 2024

View reviewed changes

marcphilipp changed the title ~~Issue: #3708 add ParameterizedTest#argumentCountValidation~~ Add ParameterizedTest#argumentCountValidation Nov 17, 2024

marcphilipp merged commit 6082336 into junit-team:main Nov 17, 2024
15 checks passed

JonasJebing mentioned this pull request Nov 18, 2024

Introduce support for zero invocations in test templates and parameterized tests #1477

Closed

marcphilipp mentioned this pull request Nov 19, 2024

Rename new @ParameterizedTest attributes to avoid confusion #4137

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ParameterizedTest#argumentCountValidation #4045

Add ParameterizedTest#argumentCountValidation #4045

JonasJebing commented Oct 4, 2024 •

edited

Loading

marcphilipp Oct 8, 2024

JonasJebing Oct 23, 2024

marcphilipp commented Oct 8, 2024

JonasJebing commented Oct 23, 2024

marcphilipp left a comment

JonasJebing commented Oct 25, 2024

marcphilipp left a comment

JonasJebing commented Nov 13, 2024

marcphilipp commented Nov 13, 2024

marcphilipp commented Nov 13, 2024

JonasJebing commented Nov 13, 2024 •

edited

Loading

marcphilipp left a comment

marcphilipp Nov 13, 2024

marcphilipp Nov 13, 2024

JonasJebing commented Nov 14, 2024

JonasJebing commented Nov 14, 2024

marcphilipp commented Nov 17, 2024

		logger.warn(() -> String.format(
		"Ignored invalid configuration '%s' set via the '%s' configuration parameter.", value, key));

Add ParameterizedTest#argumentCountValidation #4045

Add ParameterizedTest#argumentCountValidation #4045

Conversation

JonasJebing commented Oct 4, 2024 • edited Loading

Overview

Open Questions

Definition of Done

marcphilipp Oct 8, 2024

Choose a reason for hiding this comment

JonasJebing Oct 23, 2024

Choose a reason for hiding this comment

marcphilipp commented Oct 8, 2024

JonasJebing commented Oct 23, 2024

marcphilipp left a comment

Choose a reason for hiding this comment

JonasJebing commented Oct 25, 2024

marcphilipp left a comment

Choose a reason for hiding this comment

JonasJebing commented Nov 13, 2024

marcphilipp commented Nov 13, 2024

marcphilipp commented Nov 13, 2024

JonasJebing commented Nov 13, 2024 • edited Loading

marcphilipp left a comment

Choose a reason for hiding this comment

marcphilipp Nov 13, 2024

Choose a reason for hiding this comment

marcphilipp Nov 13, 2024

Choose a reason for hiding this comment

JonasJebing commented Nov 14, 2024

JonasJebing commented Nov 14, 2024

marcphilipp commented Nov 17, 2024

JonasJebing commented Oct 4, 2024 •

edited

Loading

JonasJebing commented Nov 13, 2024 •

edited

Loading