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

Migrate tests in detekt-rules-errorprone to junit #4523

Merged
merged 7 commits into from Feb 1, 2022

Conversation

marschwar
Copy link
Contributor

Part of #4450

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #4523 (a90418b) into main (0b36ff0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #4523   +/-   ##
=========================================
  Coverage     84.42%   84.42%           
  Complexity     3323     3323           
=========================================
  Files           479      479           
  Lines         11139    11139           
  Branches       2039     2039           
=========================================
  Hits           9404     9404           
  Misses          700      700           
  Partials       1035     1035           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b36ff0...a90418b. Read the comment docs.

@BraisGabin
Copy link
Member

I'm having this same issue in #4459 (https://github.com/detekt/detekt/runs/4934052976?check_suite_focus=true#step:4:556)

It seems that our compile-test-snippets is a bit flaky. If I change the number of engines here:

From 8 to 1 a lot of issues appear on main.

And, in my case if I change the number from 8 to 9 all works perfectly fine...

That PR is older than our PRs related with JUnit so JUnit is not the reason.

@3flex 3flex added the housekeeping Marker for housekeeping tasks and refactorings label Jan 25, 2022
@marschwar
Copy link
Contributor Author

marschwar commented Jan 25, 2022

Really strange...
At least it seems to be reproducable locally. Could it be possible that there are too many tests that try to access KotlinScriptEngine.compile(content) at the same time without any synchronization?

@BraisGabin
Copy link
Member

Any idea why we have 8 of those? It doesn't seem that we are doing any parallel job here so with one it should be enough.

@marschwar
Copy link
Contributor Author

No idea. It may also be something different entirely. It could well be, that I have some errors in the scripts to compile. I will investigate.

@marschwar marschwar marked this pull request as draft January 26, 2022 08:07
@marschwar
Copy link
Contributor Author

In this case it seems like there is something wrong with the snippets in IgnoredReturnValueSpec.kt

@marschwar marschwar marked this pull request as ready for review January 30, 2022 23:24
@cortinico
Copy link
Member

There is a merge conflict on this PR now. @marschwar are you able to take a look.
We also have #4541 that depends on this 👍

@marschwar marschwar merged commit c34f5fa into detekt:main Feb 1, 2022
@marschwar marschwar deleted the junit-rules-errorprone branch February 1, 2022 06:24
@cortinico cortinico added this to the 1.20.0 milestone Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants