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

Rules shared between tests have ambiguous names #452

Closed
csemrau opened this issue Oct 10, 2020 · 7 comments · Fixed by #1279
Closed

Rules shared between tests have ambiguous names #452

csemrau opened this issue Oct 10, 2020 · 7 comments · Fixed by #1279

Comments

@csemrau
Copy link

csemrau commented Oct 10, 2020

As mentioned in my other post #450, we have lots of "projects" that share a "platform", and we are restricting access to the "platform".

The "platform" provides some rules annotated as @ArchTest, e.g. "only_use_allowed_platform_classes" in a class "platform.ApiUsageRules". Each "project" declares a JUnit 4 test "projectX.ApiUsageTest", annotated to run the ArchUnitRunner, which tests the rules declared in ApiUsageRules via ArchRules.

Several "projects" are built together with Gradle on CircleCI. When one of the rules fails, the failing test is called "only_use_allowed_platform_classes - platform.ApiUsageRules", which the location where the rule was defined. That is of course the same for all "projects".

Most of the time, I could deduce from the violations which "project" is affected, but not always. From the build test output I could not see which test failed either.

Do you have suggestions how I can more easily find the actual test that failed?
Is this a problem due to our build setup, i.e. would I find the actual test class in the output or test artifacts of a simple/standard Gradle/Maven build?

I thought about adding the actual test class name ("projectX.ApiUsageTest") to the test name.

Kind regards,
Christian

@csemrau
Copy link
Author

csemrau commented Oct 16, 2020

Running our CircleCI build again, we noticed that the JUnit test report is divided into test classes with one file per class. Every "project" that runs our ApiUsageTest produces a file TEST-platform.ApiUsageRules.xml in the same folder. Building several "projects" together means that only the report from the last one is retained. If the first one failed the test, CircleCI will then report a build failure, but shows all tests green.
I don't know if it is standard that all test results are stored in the same folder, but that's what we have right now.

@csemrau
Copy link
Author

csemrau commented Oct 19, 2020

Experimentally, we created a variant of the ArchUnitRunner, which assigns test executions not to the class that declared the test rule or test method, but to the class that is annotated with the runner. This seems to work for us, because we can now easily identify which "project" failed, and the test result files are no longer clobbered.

We did this by extending the ArchRuleExecution with an actualTestClass field initialized with the testClass of the runner, and describe the execution as belonging to that class. The actual rule instance is still resolved from the testClass of the execution which is the class owning the rule field.

csemrau added a commit to csemrau/ArchUnit that referenced this issue Oct 19, 2020
run the following command
> ./gradlew :archunit-example:example-junit4:test --tests "*.ServiceRulesTest" -P example
Inspect the test result file at
> archunit-example/example-junit4/build/reports/tests/test/index.html
You will find a duplicated report for one of the projects, and the failure of the other project is lost.

Issue: TNG#452

Signed-off-by: Christian Semrau <[email protected]>
csemrau added a commit to csemrau/ArchUnit that referenced this issue Oct 19, 2020
…ad as belonging to the class that declared the rule

Resolves TNG#452

Signed-off-by: Christian Semrau <[email protected]>
csemrau added a commit to csemrau/ArchUnit that referenced this issue Oct 19, 2020
…stead as belonging to the class that declared the rule

Resolves TNG#452

Signed-off-by: Christian Semrau <[email protected]>
csemrau added a commit to csemrau/ArchUnit that referenced this issue Oct 19, 2020
run the following command
> ./gradlew :archunit-example:example-junit4:test --tests "*.ServiceRulesTest" -P example
Inspect the test result file at
> archunit-example/example-junit4/build/reports/tests/test/index.html
You will find a duplicated report for one of the projects, and the failure of the other project is lost.

Issue: TNG#452

Signed-off-by: Christian Semrau <[email protected]>
csemrau added a commit to csemrau/ArchUnit that referenced this issue Oct 19, 2020
…o longer as belonging to the class that declared the rule

Resolves TNG#452

Signed-off-by: Christian Semrau <[email protected]>
@csemrau csemrau changed the title Rule shared between tests have ambiguous names Rules shared between tests have ambiguous names Oct 19, 2020
@codecholeric
Copy link
Collaborator

Usually I would expect the report folders of different Gradle projects to be separate 🤔 Maybe that is why this problem hasn't popped up earlier. This might be something specific of your CI setup?
In the end with this one folder for all reports of all Gradle projects, wouldn't you also have this problem, if you run a test with the exact same class name in each project? Or maybe I don't understand your setup.

But in any case, since the description contains both the test class that includes the rules as well as the rules class itself (in a hierarchical form), it would probably make sense to reflect this in the file name 🤔 Just that this seems like a build tool dependent convention.

I'll try to look into your PR, why the tests break, as soon as I can find some time! And see if that is a general way that works for Gradle, Maven, IDEs, ... (where "general" is always very hard to say upfront 😉)

@codecholeric
Copy link
Collaborator

Sorry, somehow I completely dropped the ball on this, since there was no further reaction and nobody every reported the problem again 🙈 But somehow I stumbled over this ancient issue again today and decided to look into it again. I think we should always use the outermost class as the test location, but I wanted to somehow avoid confusion by not making the test look like it really is declared in that class then (e.g. I don't want the test report exactly as if it comes from a field MyArchTest.some_rule when in fact it comes from MyArchTest > ArchTests.in(Nested.class) > ArchTests.in(EvenDeeper.class) > some_rule). And I wanted JUnit 5 support behave consistently then. I tried to solve all that in #1279.

codecholeric added a commit that referenced this issue Apr 9, 2024
…on (#1279)

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`.
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
@csemrau
Copy link
Author

csemrau commented Apr 10, 2024

Thank you for resurrecting this issue! 🙏
We are still running smoothly with our customized runner subclass. However, the upgrade from 0.x to 1.2.1 highlighted what a thorn in our side it is to access package-private ArchUnit classes and class members.
I am looking forward to adapting to the improvement once we update to the next release containing this change.

@codecholeric
Copy link
Collaborator

Glad to hear that it's still gonna be useful 🙂

@csemrau
Copy link
Author

csemrau commented Jul 23, 2024

We just updated to ArchUnit 1.3.x and finally removed our custom runner.

I really like the path-like test names that are generated by default now. In the GH action report, they are presented to me like this:
PlatformUsageRules > ApiUsageRules > only_use_allowed_platform_methods (projectX.ApiUsageTest) failed
I can clearly see which project failed the build and in which way. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants