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.

Sign up for GitHub

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

Jump to bottom

@RequiresTag does not work as expected #3531

Closed
zakhenry opened this issue May 17, 2023 · 3 comments · Fixed by #3532
Closed

@RequiresTag does not work as expected #3531

zakhenry opened this issue May 17, 2023 · 3 comments · Fixed by #3532
Labels
bug 🐛 Issues that report a problem or error in the code. framework 🏗️ Pertains to the core structure and components of the Kotest framework.
Milestone
5.7

Comments

@zakhenry
Copy link
Contributor

zakhenry commented May 17, 2023 •
edited

Which version of Kotest are you using
5.6

Reopening #3274 (marked as bug but closed due to inactivity).

Expected behavior:
Adding a RequiresTag() annotation to a spec class prevents that class from being run unless the tag is in the list of active tags in the argument (i.e. not negated, or no tags specified)

Observed behavior:
Adding a RequiresTag() annotation to a spec class has no effect unless some other tag is specified (i.e. TagExpression is not null)

concrete example:
I have a tag object EndToEnd: Tag() that I don't want to run by default when ./gradlew myproject:test is run. I've added @RequiresTag("EndToEnd") to my e2e spec classes.

workaround: I could negate it with ./gradlew myproject:test -Dkotest.tags="!EndToEnd" however that seems to defeat the purpose of RequiresTag() annotation
@Kantis Kantis added bug 🐛 Issues that report a problem or error in the code. framework 🏗️ Pertains to the core structure and components of the Kotest framework. labels May 17, 2023
@Kantis
Copy link
Member

Kantis commented May 17, 2023

Related work was done in #3189, could be used as inspiration for how to test and fix this.

zakhenry added a commit to zakhenry/kotest that referenced this issue May 18, 2023
@zakhenry
fix(RequiresTagInterceptor): Add fix to handle scenario where no tags…
02367f2 
… are present in the runtime tags, but some specs are annotated with @RequiresTag()

Ref kotest#3531
@zakhenry zakhenry mentioned this issue May 18, 2023
Merged
@zakhenry
Copy link
Contributor Author

@Kantis FYI I've proposed a fix at #3532

I'm a little concerned this could be a breaking change for some, however I'll leave that call up to you folks 😁

Kantis pushed a commit to zakhenry/kotest that referenced this issue May 18, 2023
@zakhenry @Kantis
fix(RequiresTagInterceptor): Add fix to handle scenario where no tags…
141aebe 
… are present in the runtime tags, but some specs are annotated with @RequiresTag()

Ref kotest#3531
@Kantis
Copy link
Member

Kantis commented May 18, 2023 •
edited

This affects Kotest at least since v5.0.0, so the defect has been around a long while.. Perhaps it warrants a bit of a discussion on the semantic difference between @Tags and @RequiresTag and how they interact with TagExpression.

IMO it would make sense if @RequiresTag only runs on the presence of the required tag.. Whereas @Tags provides selectors for possible inclusion/exclusion.

E.g. a test annotated with@RequiresTag("windows") should not run with the tag expression !slow
A test annotated with @Tags("slow", "integration") should run on empty expression, !unit-test, and be ignored for the expression !slow

This would definitely be breaking behaviour though, since currently negated expressions will satisfy @RequiresTag. If we decide to go for it, we need to hold it until 6.0.

If we decide to just fix @RequiresTag to not be satisfied by empty tag expression I'm not sure if we can release it as a bug fix. IMO it would definitely be a bug fix, but it could break people's test setups. As mentioned, the defect has been present since at least 5.0.0

Kantis pushed a commit to zakhenry/kotest that referenced this issue May 19, 2023
@zakhenry @Kantis
fix(RequiresTagInterceptor): Add fix to handle scenario where no tags…
4d348b2 
… are present in the runtime tags, but some specs are annotated with @RequiresTag()

Ref kotest#3531
Kantis added a commit that referenced this issue Jul 26, 2023
@zakhenry @Kantis
fix(RequiresTagInterceptor): Add fix to handle scenario where no tags…
2d59bf4 
… are present in the runtime tags, but some specs are annotated with @RequiresTag() (#3532)

* test(RequiresTagInterceptor): Add tests for interceptor, with failing test for !3531

* fix(RequiresTagInterceptor): Add fix to handle scenario where no tags are present in the runtime tags, but some specs are annotated with @RequiresTag()

Ref #3531

* fixup! test(RequiresTagInterceptor): Add tests for interceptor, with failing test for !3531

---------

Co-authored-by: Emil Kantis <emil.kantis@protonmail.com>
@Kantis Kantis added this to the 5.7 milestone Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 Issues that report a problem or error in the code. framework 🏗️ Pertains to the core structure and components of the Kotest framework.
Projects
None yet
Milestone
5.7
2 participants
@Kantis @zakhenry