Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a static factory with Generic Arguments #4421

Closed
xenoterracide opened this issue Mar 24, 2025 · 3 comments
Closed

Add a static factory with Generic Arguments #4421

xenoterracide opened this issue Mar 24, 2025 · 3 comments

Comments

@xenoterracide
Copy link

xenoterracide commented Mar 24, 2025

Arguments class is not Generic, and neither is of this probably cant be changed, but I'm thinking we could do cleaner, or at least more idiomatic than this.

  static class Clients implements ArgumentsProvider {

    @Override
    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
      return Stream.of(
        Arguments.of((Function<Integer, CommonCardAccountRepository>) CommonCardAccountRepositoryWebClientDefault::new)
      );
    }
  }

this at least would not require casting

  static class Clients implements ArgumentsProvider {

    @Override
    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
      return Stream.of(
        Arguments.generic<Function<Integer, CommonCardAccountRepository>(CommonCardAccountRepositoryWebClientDefault::new)
      );
    }
  }

maybe

  static class Clients implements ArgumentsProvider {

    @Override
    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
      return Arguments.stream<Function<Integer, CommonCardAccountRepository>(Arguments.generic(CommonCardAccountRepositoryWebClientDefault::new))
      );
    }
  }

Making Arguments itself generic would be better, but I'd bet we're past that since the API is marked stable. I think this would probably require a few method options which provide something like up to 5 generic parameters.

  static class Clients implements ArgumentsProvider {

    @Override
    public Stream<? extends Arguments<Function<Integer, CommonCardAccountRepository>> provideArguments(ExtensionContext context) {
      return Stream.of(
        Arguments.of(CommonCardAccountRepositoryWebClientDefault::new)
      );
    }
  }

mostly spitballing at this point, maybe someone else could come up with a better API. jupiter 5.10.5

@marcphilipp
Copy link
Member

Thanks for raising the issue! I agree that casting to generic types is not great.

We initially decided to avoid making Arguments generic, because we'd need a variant for each number of arguments and when consuming we're using reflection anyway which only wants an Object[].

A common pattern I've seen is to use a Java record for this:

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
  return Stream.of(
    Arguments.of(new Data(CommonCardAccountRepositoryWebClientDefault::new))
  );
}

record Data(Function<Integer, CommonCardAccountRepository> function) {
}

That has the added benefit of ensuring that all argument sets use the same types.

Is that something you've tried (assuming you can use Java 16+)?

@xenoterracide
Copy link
Author

No, but that feels like a waste for 1-2 generic arguments. I suppose I could see it if you get beyond that, and certainly if you get beyond the recommended 4 max arguments.

@marcphilipp
Copy link
Member

marcphilipp commented Apr 11, 2025

Brainstorming

We discussed this issue in our last team meeting. Here are the results of our brainstorming:

Option 1

One option would be for you to use a third-party library like Vavr or Commons Lang to benefit from the tuple implementations. We have no plans of introducing such helper classes in JUnit.

Option 2

Another option would be for you to define interfaces like the following:

interface GenericArguments<T> extends Arguments {
	static <T> GenericArguments<T> of(T argument) {
		return () -> new Object[]{ argument };
	}
}
interface GenericArguments2<T1, T2> extends Arguments {
	static <T1, T2> GenericArguments2<T1, T2> of(T1 argument1, T2 argument2) {
		return () -> new Object[]{ argument1, argument2 };
	}
}
...

which you could then use like this:

@Override
public Stream<GenericArguments2<String, Integer>> provideArguments(ExtensionContext context) {
	return Stream.of(GenericArguments2.of("foo", 42), GenericArguments2.of("bar", 23));
}

Option 3

Yet another idea (courtesy of @sbrannen) would be use a record specific to your use case and have it implement Arguments:

record Data(String text, int number) implements Arguments {
	@Override
	public Object[] get() {
		return new Object[] { text, number };
	}
}

which you could then use in the return type:

@Override
public Stream<Data> provideArguments(ExtensionContext context) {
	return Stream.of(new Data("foo", 42), new Data("bar", 23));
}

This could also be done reflectively if there are many such record classes:

interface RecordArguments extends Arguments {
	@Override
	default Object[] get() {
		return Arrays.stream(getClass().getRecordComponents())
				.map(component -> ReflectionSupport.invokeMethod(component.getAccessor(), this))
				.toArray();
	}
}
record Data(String text, int number) implements RecordArguments {
}

Team decision

Overall, we think using custom records for complex argument sets provides the best user experience. On Java 15 and earlier, a third-party library providing tuple classes is a suitable workaround. Therefore, we're closing this issue for the time being.

marcphilipp added a commit that referenced this issue Apr 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants