Ktlint spotless tasks are always "up-to-date" even when there are changes

[JLLeitschuh]

I see this issue both with linting Gradle Kotlin DSL files and Kotlin Source files.

Making a change to source code after running spotless and getting a success the task seems to get stuck as always "up-to-date".

A simple example of this is an updated version of the KotlinExtensionTest::testWithHeader that I wrote. The test as it is in master currently passes 🎉 .

However, changing the test so that it looks like this:

@Test
public void testWithHeader() throws IOException {
    write("build.gradle",
            "plugins {",
            "    id 'nebula.kotlin' version '1.0.6'",
            "    id 'com.diffplug.gradle.spotless'",
            "}",
            "repositories { mavenCentral() }",
            "spotless {",
            "    kotlin {",
            "        licenseHeader('" + HEADER + "')",
            "        ktlint()",
            "    }",
            "}");

    // First run, this will put the task into it's "up-to-date" state.
    runTestWithHeader();
    // Second run should see the changes and the task should not be "up-to-date".
    runTestWithHeader();
}

private void runTestWithHeader() throws IOException {
    final File testFile = write("src/main/kotlin/test.kt", getTestResource("kotlin/licenseheader/KotlinCodeWithoutHeader.test"));
    final String original = read(testFile.toPath());
    gradleRunner().withArguments("spotlessApply").build();
    final String result = read(testFile.toPath());
    Assertions
            .assertThat(result)
            // Make sure the header gets added.
            .startsWith(HEADER)
            // Make sure that the rest of the file is still there with nothing removed.
            .endsWith(original)
            // Make sure that no additional stuff got added to the file.
            .contains(HEADER + '\n' + original);
}

Results in this test failure:

java.lang.AssertionError: 
Expecting:
 <"@file:JvmName("SomeFileName")
package my.test

object AnObject { }

">
to start with:
 <"// License Header">

    at com.diffplug.gradle.spotless.KotlinExtensionTest.runTestWithHeader(KotlinExtensionTest.java:77)
    at com.diffplug.gradle.spotless.KotlinExtensionTest.testWithHeader(KotlinExtensionTest.java:66)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.rules.RunRules.evaluate(RunRules.java:20)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

You will notice that the failure stack trace is from the second call to runTestWithHeader meaning that the second run of spotlessApply didn't reformat the source code.

[nedtwigg]

Try putting a sleep(2000) in between the two calls to runTestWithHeader(). Also, it would be simpler to assert that the content equals something, rather than startswith, endswith, and contains.

[JLLeitschuh]

Regardless of the test failing I see the behaviour in my own builds.

[JLLeitschuh]

Yes, even with the Thread.sleep the tests still fails.

[nedtwigg]

Well it's great to have a reproducible test case! Adding --debug to the gradle command line will dump to console the reasons why Gradle has determined that the files are up-to-date.

[JLLeitschuh]

Updated test code:

@Test
public void testWithHeader() throws IOException, InterruptedException {
    write("build.gradle",
            "plugins {",
            "    id 'nebula.kotlin' version '1.0.6'",
            "    id 'com.diffplug.gradle.spotless'",
            "}",
            "repositories { mavenCentral() }",
            "spotless {",
            "    kotlin {",
            "        licenseHeader('" + HEADER + "')",
            "        ktlint()",
            "    }",
            "}");

    // First run, this will put the task into it's "up-to-date" state.
    runTestWithHeader();
    Thread.sleep(2000);
    // Second run should see the changes and the task should not be "up-to-date".
    runTestWithHeader();
}

private void runTestWithHeader() throws IOException {
    final File testFile = write("src/main/kotlin/test.kt", getTestResource("kotlin/licenseheader/KotlinCodeWithoutHeader.test"));
    final String original = read(testFile.toPath());
    gradleRunner()
            .withArguments("spotlessApply", "--debug")
            .forwardOutput()
            .build();
    final String result = read(testFile.toPath());
    Assertions
            .assertThat(result)
            // Make sure the header gets added.
            .startsWith(HEADER)
            // Make sure that the rest of the file is still there with nothing removed.
            .endsWith(original)
            // Make sure that no additional stuff got added to the file.
            .isEqualTo(HEADER + '\n' + original);
}

Test results:

TestDebugLog.log.zip

[nedtwigg]

This was an excellent find. We do have have an up-to-date bug, but it's quite hard to trigger, and this test isolated it perfectly. It's not related to ktlint, it's for all our formatters.

The sequence to trigger the bug is this:

1. Have a file with bad formatting
2. Run spotlessApply to fix it
3. Manually change the file back to exactly its content in step 1
4. spotlessApply will now falsely claim to be up-to-date

If you never follow that exact sequence, you'll never see it. And to fix it, all you need to do is add a space or something and you'll be back in the proper pipeline. Surely many users (and authors!) have encountered this, but shrugged it off as a brain fart / editor saving weirdness.

I added a commit (just above) which demonstrates this more clearly. Miraculously, I've been working on another gradle plugin with weird up-to-date checking, and I think there's a hack I learned there that can fix this problem. I'll submit a PR with a fix tomorrow..

[jbduncan]

Surely many users (and authors!) have encountered this, but shrugged it off as a brain fart / editor saving weirdness.

I'm pretty sure I've encountered such behaviour with Spotless myself in the past, not knowing for sure what was causing it. Here's to hoping this resolves that little niggle. :smiley:

[jbduncan]

Although, I find it curious we even need a hack to get this working. Something to report to the Gradle team about? :thinking:

[nedtwigg]

The up-to-date API they have is great for .java -> .class. We are modifying our own inputs, which means we're asking for them to have an API for modifying what "up-to-date" means based on the result of the execution. I will definitely put the up-to-date problems (this and my image-grinder one) together to show the Gradle team as a usecase, but it wouldn't be crazy for them to decide that our usecase is too niche.

[nedtwigg]

This is currently in 3.6.0-SNAPSHOT. It's a deep enough change that I'd like to use it in integration a little before I release.

[nedtwigg]

Published as 3.6.0.

[JLLeitschuh]

Woot!!! :D Thanks for doing a deep dive on this and figuring out what the cause is. Really awesome support from you all. Awesome tool you have here. I'm always pleased with your professional nature.

[nedtwigg]

Thanks for the testcase!

[nedtwigg]

Just FYI, I've posted this and another example to the gradle forum.

[JLLeitschuh]

I would have posted that as a github issue, but sure.