Skip to content
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

Feature/invocaton index #2785

Closed
wants to merge 6 commits into from
Closed

Feature/invocaton index #2785

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 5, 2021

Overview

Relates to #1668.

This PR adds a getInvocationIndex() to ArgumentsAccessor, allowing test authors access to the index of the parameterized test.


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


Definition of Done

@codecov
Copy link

codecov bot commented Dec 5, 2021

Codecov Report

Merging #2785 (5f804a1) into main (3e06d8f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #2785   +/-   ##
=========================================
  Coverage     91.04%   91.04%           
  Complexity     4473     4473           
=========================================
  Files           385      385           
  Lines         10846    10846           
  Branches        845      845           
=========================================
  Hits           9875     9875           
  Misses          744      744           
  Partials        227      227           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e06d8f...5f804a1. Read the comment docs.

@ghost ghost marked this pull request as ready for review December 5, 2021 00:45
@@ -63,6 +67,4 @@ on GitHub.

==== New Features and Improvements

* `@TempDir` now includes a cleanup mode attribute for preventing a temporary directory
from being deleted after a test. The default cleanup mode can be configured via a
configuration parameter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving that to the right section. 👍

Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, one minor thing...

@@ -1688,6 +1688,9 @@ this API, you can access the provided arguments through a single argument passed
test method. In addition, type conversion is supported as discussed in
<<writing-tests-parameterized-tests-argument-conversion-implicit>>.

Additionally, `{ArgumentsAccessor}` makes the current index of the argument sources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the average user would know what "current index of the argument sources" means. So it's probably better to add a bit more prose here to briefly explain it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rewritten this to hopefully make it clearer.

@stale
Copy link

stale bot commented Feb 10, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions.

@stale stale bot added the status: stale label Feb 10, 2022
@stale
Copy link

stale bot commented Mar 3, 2022

This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest branch (usually main), all review comments have been addressed and the build is passing.

@stale stale bot closed this Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make invocation index for @ParameterizedTest available to test author
3 participants