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

MINOR: Introduce ApiKeyVersionsSource annotation for ParameterizedTest #11468

Merged
merged 4 commits into from Nov 11, 2021

Conversation

dajac
Copy link
Contributor

@dajac dajac commented Nov 4, 2021

It is common in our code base to have unit tests which must be run for all the versions of a given request. Most of the time, we do so by iterating over all the versions in the test itself which is error prone.

With JUnit5 and ParameterizedTest, we can now use a custom arguments source for this case, which is way more convenient. It looks likes this:

@ParameterizedTest
@ApiKeyVersionsSource(apiKey = ApiKeys.ADD_PARTITIONS_TO_TXN)
public void mytest(short version) {
  // do smth based on version...
}

This patch introduces the new annotation and updates AddPartitionsToTxnRequestTest test as a first example. I will migrate all the other cases in subsequent PRs.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dajac dajac changed the title WIP MINOR: Introduce ApiKeyVersionsSource annotation for ParameterizedTest Nov 5, 2021
@dajac dajac marked this pull request as ready for review November 5, 2021 07:35
@dajac
Copy link
Contributor Author

dajac commented Nov 8, 2021

@hachikuji What do you think about this?

Copy link
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

Great idea! LGTM

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

@dajac , thanks for the PR. Left some comments. Thanks.

public class ApiKeyVersionsProvider implements ArgumentsProvider, AnnotationConsumer<ApiKeyVersionsSource> {
private ApiKeys apiKey;

ApiKeyVersionsProvider() { }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is this constructor necessary?

import org.apache.kafka.common.protocol.ApiKeys;
import org.junit.jupiter.params.provider.ArgumentsSource;

@Target({ElementType.ANNOTATION_TYPE, ElementType.METHOD})
Copy link
Contributor

Choose a reason for hiding this comment

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

We only applied for ElementType.METHOD, so ElementType.ANNOTATION_TYPE can be removed.

@dajac
Copy link
Contributor Author

dajac commented Nov 11, 2021

@showuon Thanks for your review. I have addressed your comments.

@dajac
Copy link
Contributor Author

dajac commented Nov 11, 2021

I will merge the PR as the minor comments have been addressed.

@dajac dajac merged commit 6f62485 into apache:trunk Nov 11, 2021
@dajac dajac deleted the api-keys-source branch November 11, 2021 15:39
dajac added a commit that referenced this pull request Nov 11, 2021
…Test` (#11468)

It is common in our code base to have unit tests which must be run for all the versions of a given request. Most of the time, we do so by iterating over all the versions in the test itself which is error prone.

With JUnit5 and ParameterizedTest, we can now use a custom arguments source for this case, which is way more convenient. It looks likes this:

```
@ParameterizedTest
@ApiKeyVersionsSource(apiKey = ApiKeys.ADD_PARTITIONS_TO_TXN)
public void mytest(short version) {
  // do smth based on version...
}
```

This patch introduces the new annotation and updates `AddPartitionsToTxnRequestTest` test as a first example. I will migrate all the other cases in subsequent PRs.

Reviewers: Luke Chen <showuon@gmail.com>, Jason Gustafson <jason@confluent.io>
@dajac
Copy link
Contributor Author

dajac commented Nov 11, 2021

Merged to trunk and 3.1.

xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
…Test` (apache#11468)

It is common in our code base to have unit tests which must be run for all the versions of a given request. Most of the time, we do so by iterating over all the versions in the test itself which is error prone.

With JUnit5 and ParameterizedTest, we can now use a custom arguments source for this case, which is way more convenient. It looks likes this:

```
@ParameterizedTest
@ApiKeyVersionsSource(apiKey = ApiKeys.ADD_PARTITIONS_TO_TXN)
public void mytest(short version) {
  // do smth based on version...
}
```

This patch introduces the new annotation and updates `AddPartitionsToTxnRequestTest` test as a first example. I will migrate all the other cases in subsequent PRs.

Reviewers: Luke Chen <showuon@gmail.com>, Jason Gustafson <jason@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants