Skip to content
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

Fix spotlessFiles parameter tests #737

Merged
merged 2 commits into from
Nov 17, 2020
Merged

Fix spotlessFiles parameter tests #737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f841d27
Fix spotlessFiles parameter tests
driv Nov 15, 2020
e3b4e10
Update plugin-maven/CHANGES.md
driv Nov 17, 2020
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
2 changes: 2 additions & 0 deletions plugin-maven/CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Fixed
* Fix broken test for spotlessFiles parameter on windows ([#737](https://github.com/diffplug/spotless/pull/737))

## [2.6.0] - 2020-11-13
### Added
Expand Down
24 changes: 21 additions & 3 deletions plugin-maven/src/test/java/com/diffplug/spotless/maven/SpecificFilesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,30 @@
package com.diffplug.spotless.maven;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.junit.Test;

public class SpecificFilesTest extends MavenIntegrationHarness {
private String testFile(int number, boolean absolute) throws IOException {
String rel = "src/main/java/test" + number + ".java";
Path path;
if (absolute) {
return rootFolder() + "/" + rel;
path = Paths.get(rootFolder().getAbsolutePath(), rel);
} else {
return rel;
path = Paths.get(rel);
}
String result = path.toString();
if (!isOnWindows()) {
return result;
} else {
return result.replace("\\", "\\\\");
}
}

private boolean isOnWindows() {
return System.getProperty("os.name").startsWith("Windows");
Copy link
Member

Choose a reason for hiding this comment

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

I would use

spotless/lib/src/main/java/com/diffplug/spotless/FileSignature.java

Lines 112 to 115 in 5d965e7

/** Returns true if this JVM is running on a windows machine. */
public static boolean machineIsWin() {
return machineIsWin;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was 2 seconds away from pushing this change. :)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, guess my review was sloppy. "Is the project better now?" If the answer is yes, then I try to get people's stuff merged-in ASAP - I try to err on the side of not blocking people once I'm confident that it's not making new bugs or vulnerabilities. It's not a big deal to have this duplicated. If you want to swap this out in some future PR then fine, if it stays duplicated that's fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'll look into the this issue #710 and add it there.

}

private String testFile(int number) throws IOException {
Expand Down Expand Up @@ -73,6 +86,11 @@ public void multiFile() throws IOException, InterruptedException {

@Test
public void regexp() throws IOException, InterruptedException {
integration(".*/src/main/java/test\\(1\\|3\\).java", true, false, true);
String pattern;
if (isOnWindows())
pattern = "\".*\\\\src\\\\main\\\\java\\\\test(1|3).java\"";
else
pattern = "'.*/src/main/java/test(1|3).java'";
integration(pattern, true, false, true);
}
}