Skip to content

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

Merged
marschwar merged 7 commits into
detekt:mainfrom
marschwar:junit-rules-errorprone
Feb 1, 2022
Merged

Migrate tests in detekt-rules-errorprone to junit#4523
marschwar merged 7 commits into
detekt:mainfrom
marschwar:junit-rules-errorprone

Conversation

@marschwar
Copy link
Copy Markdown
Contributor

Part of #4450

@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

4 participants