Skip to content

Fixed some newline issues for running tests on Windows #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nedtwigg merged 4 commits into from
Dec 25, 2017
Merged

Fixed some newline issues for running tests on Windows #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c41a76
StepHarness.forStep() now ignores newline quibbles, since FormatterSt…
nedtwigg Dec 22, 2017
18c4312
ResourceHarness.read() has been doing newline reconciliation, which m…
nedtwigg Dec 22, 2017
ef19a01
To make sure that GradleIntegationTests have the same result on linux…
nedtwigg Dec 22, 2017
a0c97af
Apparently there was windows-specific behavior in GradleIncrementalRe…
nedtwigg Dec 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions plugin-gradle/src/test/java/com/diffplug/gradle/spotless/EncodingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ public void globalIsRespectedButCanBeOverridden() throws Exception {
"}");
write("test.java", "µ");
write("utf32.encoded", LineEnding.UNIX, Charset.forName("UTF-32"), "µ");
Assert.assertEquals("µ\n", read("utf32.encoded", LineEnding.UNIX, Charset.forName("UTF-32")));
Assert.assertEquals("µ\n", read("utf32.encoded", Charset.forName("UTF-32")));

gradleRunner().withArguments("spotlessApply").build();
Assert.assertEquals("??\n", read("test.java"));
Assert.assertEquals("A\n", read("utf32.encoded", LineEnding.UNIX, Charset.forName("UTF-32")));
Assert.assertEquals("A\n", read("utf32.encoded", Charset.forName("UTF-32")));
}
}
9 changes: 3 additions & 6 deletions ...in-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@

import com.diffplug.common.base.Errors;
import com.diffplug.common.base.StringPrinter;
import com.diffplug.spotless.LineEnding;

public class GradleIncrementalResolutionTest extends GradleIntegrationTest {
private static final boolean IS_UNIX = LineEnding.PLATFORM_NATIVE.str().equals("\n");

@Test
public void failureDoesntTriggerAll() throws IOException {
write("build.gradle",
Expand Down Expand Up @@ -60,9 +57,9 @@ public void failureDoesntTriggerAll() throws IOException {
checkRanAgainst("abc");
// apply will run against all three the first time
applyRanAgainst("abc");
// for some reason, it appears unix has higher resolution on which files need to be checked
applyRanAgainst(IS_UNIX ? "b" : "abc");
// but nobody the last time
// the second time, it will only run on the file that was changes
applyRanAgainst("b");
// and nobody the last time
applyRanAgainst("");

// if we change just one file
Expand Down
6 changes: 6 additions & 0 deletions plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.gradle.testkit.runner.GradleRunner;
import org.gradle.testkit.runner.TaskOutcome;
import org.junit.Assert;
import org.junit.Before;

import com.diffplug.common.base.Errors;
import com.diffplug.common.base.StringPrinter;
Expand All @@ -36,6 +37,11 @@
import com.diffplug.spotless.ResourceHarness;

public class GradleIntegrationTest extends ResourceHarness {
@Before
public void gitAttributes() throws IOException {
write(".gitattributes", "* text eol=lf");
}
Copy link
Member

@jbduncan jbduncan Dec 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not so clear on what writing this .gitattributes file does. What do the other tests do with it?

Copy link
Member Author

@nedtwigg nedtwigg Dec 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default line ending behavior is to match git. We have a .gitattributes file so that spotless will checkout with \n on every platform.

Our GradleIntegrationTests create a folder in a temp folder, and run a gradle build there. Because those test folders don't have a .gitattributes file, git (on windows) will default to \r\n. So now if you read a test file from the spotless test resources, and compare it to a build result, the line endings won't match. We used to fix this in ResourceHarness by switching line endings to UNIX, which was dangerous. Now we do it buy sticking this .gitattributes file into the test directory.

Copy link
Member

@jbduncan jbduncan Dec 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, that makes sense to me now. Thanks for clarifying things for me!

In that case, this whole PR LGTM. 👍


protected GradleRunner gradleRunner() throws IOException {
return GradleRunner.create().withProjectDir(rootFolder()).withPluginClasspath();
}
Expand Down
25 changes: 7 additions & 18 deletions testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,31 +110,20 @@ protected AbstractFileAssert<?> assertFile(String path) throws IOException {
return Assertions.assertThat(newFile(path)).usingCharset(StandardCharsets.UTF_8);
}

protected String read(Path path) throws IOException {
return read(path, LineEnding.UNIX);
}

protected String read(String path) throws IOException {
return read(path, LineEnding.UNIX);
return read(newFile(path).toPath());
}

protected String read(String path, LineEnding ending) throws IOException {
return read(path, ending, StandardCharsets.UTF_8);
}

protected String read(Path path, LineEnding ending) throws IOException {
return read(path, ending, StandardCharsets.UTF_8);
protected String read(Path path) throws IOException {
return read(path, StandardCharsets.UTF_8);
}

protected String read(String path, LineEnding ending, Charset encoding) throws IOException {
Path target = newFile(path).toPath();
return read(target, ending, encoding);
protected String read(String path, Charset encoding) throws IOException {
return read(newFile(path).toPath(), encoding);
}

protected String read(Path path, LineEnding ending, Charset encoding) throws IOException {
String content = new String(Files.readAllBytes(path), encoding);
String allUnixNewline = LineEnding.toUnix(content);
return allUnixNewline.replace("\n", ending.str());
protected String read(Path path, Charset encoding) throws IOException {
return new String(Files.readAllBytes(path), encoding);
}

protected void replace(String path, String toReplace, String replaceWith) throws IOException {
Expand Down
5 changes: 4 additions & 1 deletion testlib/src/main/java/com/diffplug/spotless/StepHarness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ public StepHarness(FormatterFunc formatter) {

/** Creates a harness for testing steps which don't depend on the file. */
public static StepHarness forStep(FormatterStep step) {
return new StepHarness(input -> step.format(input, new File("")));
// We don't care if an individual FormatterStep is misbehaving on line-endings, because
// Formatter fixes that. No reason to care in tests either. It's likely to pop up when
// running tests on Windows from time-to-time
return new StepHarness(input -> LineEnding.toUnix(step.format(input, new File(""))));
}

/** Creates a harness for testing a formatter whose steps don't depend on the file. */
Expand Down