[jvm] Using selfie with Gradle Develocity Test Distribution

[glasser]

We use Gradle's "Develocity" test distribution which runs our JVM tests in our Kubernetes cluster rather than on dev laptops. This isn't compatible with Selfie's checkout-rewriting functionality.

As a workaround, we've added an extra Test-type task to our projects that disables test distribution and sets selfie=overwrite, and we've made our normal tests set selfie=readonly (though the latter may be overkill). This does mean that you need to take an explicit action and can't just use the selfieonce/SELFIEWRITE features.

Has anyone else run into this? We're thinking of writing something that looks for the magic comments and automatically disables test distribution if they exist, and are curious if anyone else has come up with this already.

[nedtwigg]

we've made our normal tests set selfie=readonly (though the latter may be overkill)

That seems like a good precaution to take. I haven't used Develocity test distribution, if it sets CI=true then selfie will already be running in readonly mode, but if it doesn't set CI you would definitely want an error if //selfieonce was modifying test code that was destined to get silently thrown away in some k8s cluster.

thinking of writing something that looks for the magic comments and automatically disables test distribution

Fwiw, I'd be happy to merge a static CommentTracker.hasComment(fileContent: String) method so that you can be guaranteed that Selfie and plugin logic remain in sync.

Right now comment detection is as simple as this:

selfie/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/CommentTracker.kt

Line 60 in 319a1b9

for (comment in listOf("//selfieonce", "// selfieonce", "//SELFIEWRITE", "// SELFIEWRITE")) {

But we have an open PR which handles string constants and multiline comments. I'm leaning against merging it due to the compexity it adds, but adding a public hasComment method would put that problem safely into a box.

• Off-by-one in the error message for a VCR key mismatch. #526

isn't compatible with Selfie's checkout-rewriting functionality

The real magic would be to aggregate the deltas across the test runners and bring them back to whoever ran the tests. Javascript snapshot testing already has to deal with this for running tests within the browser, but Selfie has no such mechanisms. I'd be happy to merge PRs which added support for this, even if it comes at the cost of some complexity to selfie's core.

[glasser]

Fwiw, I'd be happy to merge a static CommentTracker.hasComment(fileContent: String) method so that you can be guaranteed that Selfie and plugin logic remain in sync.

We'd probably want to detect _TODO( as well, right?

For now, we're going with this in relevant Gradle project:

fun isCi(): Boolean {
	return System.getenv("CI") != null
}

tasks.test {
	if (!isCi()) develocity { testDistribution { enabled = false } }

	inputs.files(fileTree("src/test/kotlin") {
		include("**/*.kt")
		include("**/*.ss")
	})
}

This means that CI will still be able to use test distribution (in readonly mode) and local execution will always run locally. (Throwing in a scan for comments here could work too.)

Note that while the docs say that setting inputs.files is optional, it actually appears to be necessary with test distribution — otherwise the snapshot files don't end up sent to the test runner server. It also looks like specifying sending over the Kotlin files is necessary even in readonly mode (I think these files are inputs to compileTestKotlin rather than test itself normally) — even in readonly module, system.sourceFileHasWritableComment gets called to throw an error by toMatchDisk via canWrite. (Maybe this is a bug, or there could be a selfie=source-unavailable that doesn't even try to check for the comments? nbd to include the source though via the line above.)

[nedtwigg]

We'd probably want to detect _TODO( as well, right?

Ah, good point...

there could be a selfie=source-unavailable

This would make including the .kt files optional, but the .ss files would still be required, so it seems like only a marginal improvement. But if you find the mode valuable and it doesn't add much complexity I'd be happy to merge it.

I'd be happy to accept complexity for some kind of selfie=source-remote which allowed full distributed test running.

[glasser]

Yeah, it's probably better to just send everything over (with docs).