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

Random failures: test success but should not #395

Closed
uhafner opened this issue Jul 3, 2020 · 9 comments
Closed

Random failures: test success but should not #395

uhafner opened this issue Jul 3, 2020 · 9 comments

Comments

@uhafner
Copy link

uhafner commented Jul 3, 2020

I have a test that checks that some specific static calls are not used in my code: PluginArchitectureTest.java#L33. I introduced a violation of this rule in my PR, see IssuesChartPortlet.java#L54.

When I use this rule it works perfectly in a VM and in GitHub Actions, but the rule fails on my local machine (MacBook) or sometimes in our CI (see PR jenkinsci/warnings-ng-plugin#546 and CI results (Windows fails, Linux succeeds).

I'm not sure if I use some concepts in the wrong way or if this is a bug that occurs only sometimes.

E.g. if I use the annotation @AnalyzeClasses(packages = "io.jenkins.plugins.analysis") then the test succeeds on my machine. If I restrict the scanned packages to @AnalyzeClasses(packages = "io.jenkins.plugins.analysis.core.portlets") then the test correctly fails. Is this a concurrency problem? Or is the usage of the static fields for the tests a problem? I tried to debug this issue but actually I am a bit of lost when the rules are dynamically invoked. Maybe you can give me some hints on how to find the root cause for this problem.

@codecholeric
Copy link
Collaborator

My first instinct would be that this is a classpath problem 🤔 Something in the direction of #319.
I might help to print the classpath within the test to easily compare how it looks when it fails and when it succeeds. That might also explain, why a subpackage might get found, but if you use the parent package it doesn't. Although I thought I fixed this with version 0.14.0 (and AFAIS you're using the current version, right?)
You could also compare it to the DEBUG log output of com.tngtech.archunit.core.importer.UrlSource. And last but not least, you could add a simple method

@ArchTest
static void printClasses(JavaClasses classes) {
    assertThat(classes.contain("io.jenkins.plugins.analysis.core.portlets.IssuesChartPortlet")).isTrue();
}

to make sure that the class that you expect to violate the rule is actually imported.

@codecholeric
Copy link
Collaborator

Did you manage to find the reason? Is there anything we need to fix in ArchUnit?

@uhafner
Copy link
Author

uhafner commented Sep 6, 2020

Sorry, I forgot to dig into it any further.

Now I added the logging you mentioned but the class seems to be loaded. I added another test in the PR so that you see the results of both tests (one with the sub package and one with the top level package). So I think there is something wrong (or I am using the API in the wrong way?).

@uhafner
Copy link
Author

uhafner commented Sep 6, 2020

I added the logging output of the class paths:
classpath.1.txt
classpath.2.txt
And the whole log:
everything.log
Maybe it helps?

@uhafner
Copy link
Author

uhafner commented Sep 6, 2020

I put my code in the PR jenkinsci/warnings-ng-plugin#546.

@codecholeric
Copy link
Collaborator

Oh man, I completely lost track of this. I guess you didn't find out the reason? I actually don't see a classpath problem and I have trouble to run this locally. One thing I have noticed from the linked PR is though, that there was a change where before it was

@AnalyzeClasses(packages = "io.jenkins.plugins.analysis..")

I don't think you can use the package matcher syntax here, i.e. ... It also would not make sense, because ArchUnit automatically imports sub-packages with the ClassFileImporter, you can't just import a package without sub-packages with this API.
But even then, for the current state I would expect both PluginArchitectureTest and PluginArchitectureTest2 to fail, because both define the same rule and import both classes that form the violation 🤔
Too bad the project doesn't build on my machine at all 😞

@uhafner
Copy link
Author

uhafner commented Dec 4, 2020

Hmm, I should have written the comment of jenkinsci/warnings-ng-plugin#546 here:

After merging with master now both tests are failing. This means, ArchUnit is now correctly running!

I really have no idea what caused the problem. Maybe something related to Jenkins complex class loader structure or dependencies. So I think I we can close #395. If it happens again I'll try to provide a simpler way to reproduce.

@uhafner uhafner closed this as completed Dec 4, 2020
@uhafner
Copy link
Author

uhafner commented Dec 4, 2020

I also remove the 2 dots in the @AnalyzeClasses(packages = "io.jenkins.plugins.analysis..") annotations now in all my tests. Seems to work in all my plugins in HEAD.

@codecholeric
Copy link
Collaborator

Okay, let me know if there are further problems! 😃

hankem added a commit that referenced this issue Dec 9, 2022
Bumps [actions/setup-java](https://github.com/actions/setup-java) from
3.6.0 to 3.8.0.

from [actions/setup-java's releases](https://github.com/actions/setup-java/releases):

> # v3.8.0
> 
> In scope of this release we added logic to pass the token input through on GHES for Microsoft Build of OpenJDK ([actions/setup-java#395](https://github-redirect.dependabot.com/actions/setup-java/pull/395)) and updated [minimatch](https://github-redirect.dependabot.com/actions/setup-java/pull/413) dependency.

Commits

*   [`c3ac5dd`](actions/setup-java@c3ac5dd) Revert "Add support for Oracle JDK ([#401](https://github-redirect.dependabot.com/actions/setup-java/issues/401))" ([#421](https://github-redirect.dependabot.com/actions/setup-java/issues/421))
*   [`dcd29da`](actions/setup-java@dcd29da) Fix typo in README.md ([#419](https://github-redirect.dependabot.com/actions/setup-java/issues/419))
*   [`19eeec5`](actions/setup-java@19eeec5) Update to latest `actions/publish-action` ([#411](https://github-redirect.dependabot.com/actions/setup-java/issues/411))
*   [`bd7e5d2`](actions/setup-java@bd7e5d2) Update minimatch to 3.1.2 ([#413](https://github-redirect.dependabot.com/actions/setup-java/issues/413))
*   [`6cdf39a`](actions/setup-java@6cdf39a) Add support for Oracle JDK ([#401](https://github-redirect.dependabot.com/actions/setup-java/issues/401))
*   [`7db6b45`](actions/setup-java@7db6b45) Eclipse Temurin instead of Adopt OpenJDK ([#398](https://github-redirect.dependabot.com/actions/setup-java/issues/398))
*   [`bf2f02c`](actions/setup-java@bf2f02c) Pass the token input through on GHES for Microsoft Build of OpenJDK ([#395](https://github-redirect.dependabot.com/actions/setup-java/issues/395))
*   See full diff in [compare view](actions/setup-java@v3.6.0...v3.8.0)
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

No branches or pull requests

2 participants