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

[Junit5 extension] add dedicated Annotations for Number, Boolean and String flags #923

Open
aepfli opened this issue Aug 20, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@aepfli
Copy link
Member

aepfli commented Aug 20, 2024

Problem

Our annotations for testing are excellent, and they greatly ease the developer experience. Currently, all the values need to be provided as a string. This is a limitation of java annotation as java annotation members can only be (https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1):

  • A primitive type
  • String
  • Class or an invocation of Class (§4.5)
  • An enum type
  • An annotation type
  • An array type whose component type is one of the preceding types

Furthermore, annotations can't be generic, nor do they support inheritance.

@Flag(name = "BOOLEAN_FLAG", value = "true")

Goal

Evaluate the feasibility and effort of using meta-annotations or some kind of inheritance.
We should not make the API more complex than it already is.
Maybe we only support those convenience annotations for the simple annotation approach and not in the extended configuration.

like instead of

@Flag(name = "BOOLEAN_FLAG", value = "true")
@Flag(name = "STRING_FLAG", value = "true")
@Flag(name = "Number_FLAG", value = "1")

we would love to use

@BooleanFlag(name = "BOOLEAN_FLAG", value = true)
/* optional boolean improvement
@BooleanFlagTrue(name = "BOOLEAN_FLAG")
@BooleanFlagFalse(name = "BOOLEAN_FLAG")
*/
@StringFlag(name = "STRING_FLAG", value = "true")
@NumberFlag(name = "Number_FLAG", value = 1)
@aepfli aepfli added the enhancement New feature or request label Aug 20, 2024
@toddbaert toddbaert added the help wanted Extra attention is needed label Sep 5, 2024
@aepfli
Copy link
Member Author

aepfli commented Sep 19, 2024

JUnitPioneer is actually using something like this within their cartesian test approach, which might be worth looking at https://github.com/junit-pioneer/junit-pioneer/blob/main/src/main/java/org/junitpioneer/jupiter/cartesian/CartesianTest.java#L115-L167

@toddbaert
Copy link
Member

@UtkarshSharma2612 you might like this.

@ssharaev
Copy link
Member

Hi, @aepfli
I would like to implement this if it’s still free.
I had a look at JUnit CartesianTest and I want to make sure I understand your idea correctly.
I think we can create annotations like this:

public @interface BooleanFlag {
    /**
     * The key of the FeatureFlag.
     */
    String name();

    /**
     * The value of the FeatureFlag.
     */
    boolean value();
}

And add them in extended configuration annotation like the CartesianTest values:

public @interface OpenFeature {
    /**
     * the provider domain used for this configuration.
     */
    String domain() default "";
    /**
     * Collection of {@link Flag} configurations for this domain.
     */
    Flag[] value();

    BooleanFlag[] booleanFlags() default {};
    StringFlag[] stringFlags() default {};
	// ... other flag annotations
}

This way we can support this annotations for both simple and extended configuration, but we need to decide how to solve collisions between different flags.
Does it seem reasonable?

@aepfli
Copy link
Member Author

aepfli commented Dec 28, 2024

hey seems reasonable to me - i suggest we throw an error, if we see that the same flag key is present in both configurations.

@ssharaev
Copy link
Member

Great!
I’ll implement it and create a PR in a few days. Could you assign this issue to me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants