Migrate detekt-rules-style tests to JUnit

[3flex]

#4450

[marschwar]

Is there anything I can do to support? Monkey work is my specialty ;) .

[3flex]

That's ok, I'll try and wrap it up this weekend! week! Thanks though.

[codecov]

Codecov Report

Merging #4575 (fe6f3e3) into main (afdb999) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #4575   +/-   ##
=========================================
  Coverage     84.58%   84.58%           
  Complexity     3434     3434           
=========================================
  Files           492      492           
  Lines         11329    11329           
  Branches       2058     2058           
=========================================
  Hits           9583     9583           
  Misses          701      701           
  Partials       1045     1045           

---

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 afdb999...fe6f3e3. Read the comment docs.

[3flex]

👀

[marschwar]

How about this?

class ObjectLiteralToLambdaSpec() {
    val subject = ObjectLiteralToLambda()

    @Nested
    @KotlinCoreEnvironmentTest
    inner class DefaultSourceTests(val env: KotlinCoreEnvironment) {
      // ...
    }

    @Nested
    @KotlinCoreEnvironmentTest(additionalJavaSourcePaths = ["java"])
    inner class JavaSourceTests(val env: KotlinCoreEnvironment) {
      // ...
    }
}

[marschwar]

It would be really good, If test authors do not have to create and dispose the environment manually

[3flex]

It would be really good, If test authors do not have to create and dispose the environment manually

Agree though out of the ~1300 style tests there are only 4 using this setup, so it's very much an exception, so perhaps the manual option is OK?

As for options, I did try what you suggested above, but annotation parameters cannot be nullable, so this would mean every time the annotation is used you'd have to pass an empty list to the annotation.

We could instead have a separate annotation like @KotlinCoreEnvironmentTestWithJavaRoot that accepts the parameter. It feels awkward though to use just for these few cases, but it would be good to have a general solution for including Java sources in a test like this.

I'll wait for feedback.

[marschwar]

Annotation parameters May not be null, but they can have defaults. I did get this to work but the Code is not ready to be commited. I think it is fine to merge this and come back to it later.

[3flex]

👀

[marschwar]

I was going to suggest to extend the annotation to something like @KotlinCoreEnvironmentTest(additionalJavaSourcePaths = ["java"]). But something seems to be wrong with the test as it does not fail even when the regular env is used.

[3flex]

it does not fail even when the regular env is used.

I noticed this too, but I only picked up that that I didn't use the custom environment here when the coverage report showed a change in coverage for the rule under test.

Agree this doesn't seem to be testing what it appears to be at first glance, but fixing that can be done another time.

[marschwar]

Thank you for taking this. I did not realize how many style rules there are.

[chao2zhang]

Approve because

1. Coverage does not seem to change 84.58% -> 84.58%
2. All checks are passing
3. Scroll through the PR briefly and did not find undesired changes.
   I don't think it's worth time reading every line of code changes

[marschwar]

Any objections to merging this?

[chao2zhang]

Not from my side. Let's do it.