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

make tests included via ArchTests report outer class as test location #1279

Conversation

codecholeric
Copy link
Collaborator

@codecholeric codecholeric commented Mar 26, 2024

So far, if we include rules from other test classes, e.g.

@AnalyzeClasses(..)
class ExampleTest {
  @ArchTest
  static final ArchTests nested = ArchTests.in(Other.class);
}

then the included class (Other.class in this case) will be used as test location.
This can cause confusion, because by this if Other.class is included in two separate test classes,
analyzing different locations via @AnalyzeClasses,
the results will be reported for the same test class Other.class.
We now report the outermost class that started the test (the one with `@AnalyzeClasses)
as the test location for both JUnit 4 and JUnit 5.
To make it easy to understand through which path a rule is included in more complex scenarios,
we extend the display name to include the path of nested classes.

Resolves: #452

@codecholeric codecholeric force-pushed the make-tests-included-via-ArchTests-report-outer-class-as-test-location branch 2 times, most recently from 8920f0c to fa822f5 Compare March 27, 2024 14:24
Copy link
Member

@hankem hankem left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

With this change, test reports show where ArchTests have been included.
Comparison of old and new result for RuleLibraryTest:

effect-of-PR1279

@codecholeric codecholeric force-pushed the make-tests-included-via-ArchTests-report-outer-class-as-test-location branch from fa822f5 to a54ca34 Compare April 9, 2024 21:45
We renamed `ArchRules` to `ArchTests` a while ago to avoid confusion.
The renamed methods are some leftovers from the time before this renaming
that are now consolidated.

Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
So far, if we include rules from other test classes, e.g.
```
@RunWith(ArchUnitRunner.class)
@AnalyzeClasses(..)
class ExampleTest {
  @archtest
  static final ArchTests nested = ArchTests.in(Other.class);
}
```
then the nested class (`Other.class` in this case) will be used as test location.
This can cause confusion, because by this if `Other.class` is included in two separate test classes,
analyzing different locations via `@AnalyzeClasses`,
the results will be reported for the same test class `Other.class`.
We now set the root class that started the test as the test class of the `Description`.
To make it easy to understand through which path a rule is included in more complex scenarios,
we extend the display name to include the path of nested classes.

Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
So far, if we include rules from other test classes, e.g.
```
@AnalyzeClasses(..)
class ExampleTest {
  @archtest
  static final ArchTests nested = ArchTests.in(Other.class);
}
```
then the nested class (`Other.class` in this case) will be used as test location.
This can cause confusion, because by this if `Other.class` is included in two separate test classes,
analyzing different locations via `@AnalyzeClasses`,
the results will be reported for the same test class `Other.class`.
At least for Gradle the reason it picks `Other.class` seems to be
that it traverses up the `TestDescriptor` hierarchy
and picks the first descriptor with an attached `ClassSource`.
We can thus simply not set a `TestSource` for the intermediary descriptors,
i.e. the container descriptors caused by `ArchTests.in(..)`,
and by that make Gradle traverse up until it finds the root that started the test.
To make it easy to understand through which path a rule is included in more complex scenarios,
we extend the display name to include the path of nested classes.

Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
@codecholeric codecholeric force-pushed the make-tests-included-via-ArchTests-report-outer-class-as-test-location branch from a54ca34 to e658be0 Compare April 9, 2024 22:18
@codecholeric codecholeric merged commit 72c2f80 into main Apr 9, 2024
21 checks passed
@codecholeric codecholeric deleted the make-tests-included-via-ArchTests-report-outer-class-as-test-location branch April 9, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rules shared between tests have ambiguous names
2 participants