compile-test-snippets is confusing for new contributors

[BraisGabin]

### A bit of context:

Our unit tests contain a lot of code snippets. Because, in general, we don't compile that code, it's easy to write code that doesn't compile. That makes that our tests doesn't test what we want. To avoid this problem we compile all the snippets to ensure that they are valid.

Problem: This extra step consumes a lot of execution time making our tests even more slow.

Solution: Because this check is testing the test code (and it's not testing production code) it's ok if we don't run it every time. So we just check it on CI.

Current state

But that solution has a problem! The new contributors are bitten by this on CI and they don't know what's going on (an example is #4872 but we had a lot of these in the past).

One possible solution

We could configure a "bot" to write a comment explaining this when the workflow compile-test-snippets fails. This will give fast feedback to the contributor about what's going on, why we have this "strange" behaviour and how s/he can reproduce it locally.

This is an issue and it's not a PR because the actions that I found to add that comment doesn't work when the PRs come from fork repository. And that's exactly the case when we want to add that comment.

[cortinico]

We could configure a "bot" to write a comment explaining this when the workflow compile-test-snippets fails. This will give fast feedback to the contributor about what's going on, why we have this "strange" behaviour and how s/he can reproduce it locally.

Follow up here: This is yet another thing than can be easily automated with Danger (see more on this here #4696 (reply in thread)).

Essentially, once the CI results are complete, this can be one of the message reports on the PR by danger. Something along the line of "hey the compile-test-snippets CI job failed. This is because bla bla bla. You can test locally with bla bla bla".

[dzirbel]

A near-ideal solution might be to have compile-snippet-tests be enabled by default when running a single test but not the entire test suite; that would make individual changes easier to test with minimal impact and wouldn't slowing down the entire build. I've never tried doing something quite like that, so I'm not sure if it's possible - but might be if we can access the JUnit filters at test runtime.

[cortinico]

be enabled by default when running a single test but not the entire test suite; that would make individual changes easier to test with minimal impact and wouldn't slowing down the entire build. I've never tried doing something quite like that, so I'm not sure if it's possible

That's not really easy as, I suppose, you're running a single test inside the IDE. That's at least what I do. So you'll anyway have to instruct the user on how to configure the run config for that specific test

[dzirbel]

be enabled by default when running a single test but not the entire test suite; that would make individual changes easier to test with minimal impact and wouldn't slowing down the entire build. I've never tried doing something quite like that, so I'm not sure if it's possible

That's not really easy as, I suppose, you're running a single test inside the IDE. That's at least what I do. So you'll anyway have to instruct the user on how to configure the run config for that specific test

Yep, that's the same situation I'd like to catch; what I'd ideally like to see is a way to automatically capture the running test context and adjust the snippet compilation accordingly. It's not out of the question, for example:

@BeforeEach
fun init(testInfo: TestInfo) {
    val isRunningSingleTest = testInfo.testClass.isPresent || testInfo.testMethod.isPresent
}

I believe will be able to capture only test classes/methods run from the IDE (or from the CLI with some arcane configuration I can never remember). But of course we don't want to have this logic in each test; whether there's some way to access this data from RuleExtensions.shouldCompileTestSnippets is what I don't know.

[cortinico]

That is doable I think, yet I think that modifying the env between running 1 or N tests is not the best practice. It might create frustrations during local development and generally confuse new contributors.

[BraisGabin]

I understand the idea and it have sense but I agree with Nico here. It could create even more confusion. Why does a test work if I run ./gradlew test but it doesn't when I run it on my IDE?

Something similar could be asked about local/CI but I think that we could clarify that easier.

[dzirbel]

Yeah, I agree it can be confusing... on the other hand, so is the current practice 🤷 Just brainstorming some ideas, here are a couple more:

• create a new gradle task/set of tasks like testWithSnippetsCompiled (and maybe buildWithSnippetsCompiled, etc to match); that might be slightly more discoverable than the system parameter but also complicates the build configuration
• add an optional parameter on compileAndLint which acts as an override for compiling snippets; again this adds some discoverability and makes it a bit easier to change one-off, either by changing the default value or passing it in, but we'd have to be careful that we never commit any such changes

[dimsuz]

I'd like to add that one more thing that got me a bit confused while reading detekt source code is that project property compile-test-snippets gets transformed into a compile-snippet-tests — notice the swapping of last two words.

Relevant lines are in the same file: one, two.

Setting the first one will only work in detekt repository and won't work in rule authors' repostories, but setting the system property with the latter name should work in other repositories too (it's actually being read by detekt-test).

Maybe uniting them somehow could help here.

[cortinico]

Maybe uniting them somehow could help here.

Fixed here #5630

That's however out of the scope of this issue, as the underlying problem is that users don't understand that we have an 'extra' step where we effectively compile the test snippets source code, and that's running only under certain conditions.