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

Allow @AnalyzeClasses to be used as a meta annotation #182

Open
bobtiernay-okta opened this issue Jun 16, 2019 · 8 comments · May be fixed by #1300
Open

Allow @AnalyzeClasses to be used as a meta annotation #182

bobtiernay-okta opened this issue Jun 16, 2019 · 8 comments · May be fixed by #1300

Comments

@bobtiernay-okta
Copy link

bobtiernay-okta commented Jun 16, 2019

Having @AnalyzeClasses allowed to be a meta annotation would allow one to encapsulate repeating options across tests:

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@AnalyzeClasses(packages = {PACKAGE_X, PACKAGE_Y}, importOptions = {DoNotIncludeTests.class, DoNotIncludeJars.class, DoNotIncludeArchives.class})
public @interface AnalyzeStandardClasses

For similar features in Spring, checkout https://docs.spring.io/spring/docs/current/spring-framework-reference/testing.html#integration-testing-annotations-meta.

@bobtiernay-okta bobtiernay-okta changed the title Allow @AnalyzeClasses to be a meta annotation Allow @AnalyzeClasses to be used as a meta annotation Jun 16, 2019
@codecholeric
Copy link
Collaborator

Sounds like a reasonable addition 😃 Would be worth it to ponder about the implications if several @AnalyzeClasses come together then. I.e.

@AnalyzeUi
@AnalyzeService
class MyGeneralArchRules {
    @ArchTest
    static final ArchRule firstRule = // ...
}

Because up to now this scenario was impossible by design. I guess since all the members of @AnalyzeClasses are varargs anyway, it would be easy to just aggregate all @AnalyzeClasses within the hierarchy and add all the varargs together. This means addition of locations to import and intersection of ImportOptions though...
Also the use of two different cache modes would be a question to solve, who would win if one @AnalyzeClasses wants to clear the cache after the run and one wants to keep it? The safe thing would be to use the shorter one I guess.

I'll see if anyone wants to pitch in a PR here since I have to work on some other issues first.

@Maiklins
Copy link

Is this something that can still be picked up? Then I would give it a go.

@codecholeric
Copy link
Collaborator

Yes, I don't think anybody is actively working on this, so if you want to give it a shot I'm happy to support you where necessary 🙂

@mathze
Copy link

mathze commented May 4, 2024

As it seems that @Maiklins didn't found the time to take over, I would be happy to give it a try.
Would it be ok for you @codecholeric?

mathze added a commit to mathze/ArchUnit that referenced this issue May 4, 2024
so far users are forced to repeat `@AnalyzeClasses` annotation an every test class.
This cause additional maintenance overhead when common properties (e.g. package structure) changes.
To support the DRY approach, `@AnalzyeClasses` annotation can now be used as meta annotation.

Resolves: TNG#182
Signed-off-by: Mathze <[email protected]>
@mathze
Copy link

mathze commented May 6, 2024

Didn't got any response so I assume no one is working at this. So I've created #1300

@Maiklins
Copy link

Maiklins commented May 7, 2024

@mathze I did not find the time to start on that one. So go ahead. :)

@mathze
Copy link

mathze commented May 11, 2024

Note: My PR only allows the plain use as meta annotation. Atm. it does not respect merging nor inheritance from super classes.
Imo support for merging and inheritance should be pulled out in a separate feature request, as it requires a quite more elaborated logic, which requires further discussion, shouldn't it?

mathze added a commit to mathze/ArchUnit that referenced this issue May 11, 2024
so far users are forced to repeat `@AnalyzeClasses` annotation an every test class.
This cause additional maintenance overhead when common properties (e.g. package structure) changes.
To support the DRY approach, `@AnalzyeClasses` annotation can now be used as meta annotation.

Resolves: TNG#182
Signed-off-by: Mathze <[email protected]>
@mathze
Copy link

mathze commented May 25, 2024

@codecholeric did not received a response so far. Can you have a look at my PR #1300, please?

mathze added a commit to mathze/ArchUnit that referenced this issue Jun 25, 2024
so far users are forced to repeat `@AnalyzeClasses` annotation an every test class.
This cause additional maintenance overhead when common properties (e.g. package structure) changes.
To support the DRY approach, `@AnalzyeClasses` annotation can now be used as meta annotation.

Resolves: TNG#182
Signed-off-by: Mathze <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants