Merge pull request #31 from naver/ksmail13/fix/path-normalize

#30 file path normalize

.github/workflows/maven.yml

@@ -4,7 +4,7 @@ name: cover-checker test and get coverage
 on:
   pull_request:
     branches: [ master ]
-    types: [opened, synchronize, reopened, edited, ready_for_review, review_requested]
+    types: [opened, synchronize, reopened, edited, ready_for_review]
 
 jobs:
   test-linux:
@@ -21,7 +21,7 @@ jobs:
       shell: bash
       env:
         ACCESS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      run: java -jar ./cover-checker-console/target/cover-checker-console-1.4.1-jar-with-dependencies.jar -c ./cover-checker-cobertura/target/site/jacoco -c ./cover-checker-console/target/site/jacoco -c ./cover-checker-core/target/site/jacoco/ -c ./cover-checker-github/target/site/jacoco -c ./cover-checker-jacoco/target/site/jacoco -t 80 -r $GITHUB_REPOSITORY -g $ACCESS_TOKEN -p $(echo $GITHUB_REF | awk 'BEGIN { FS = "/" } ; { print $3 }')
+      run: java -jar ./cover-checker-console/target/cover-checker-console-1.4.2-jar-with-dependencies.jar -c ./cover-checker-cobertura/target/site/jacoco -c ./cover-checker-console/target/site/jacoco -c ./cover-checker-core/target/site/jacoco/ -c ./cover-checker-github/target/site/jacoco -c ./cover-checker-jacoco/target/site/jacoco -t 80 -r $GITHUB_REPOSITORY -g $ACCESS_TOKEN -p $(echo $GITHUB_REF | awk 'BEGIN { FS = "/" } ; { print $3 }')
   test-macos:
     runs-on: macos-latest
     steps:

cover-checker-cobertura/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <artifactId>coverchecker</artifactId>
         <groupId>com.naver.nid</groupId>
-        <version>1.4.1</version>
+        <version>1.4.2</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

cover-checker-cobertura/src/main/java/com/naver/nid/cover/cobertura/CoberturaCoverageReportHandler.java

@@ -19,10 +19,13 @@
 import com.naver.nid.cover.parser.coverage.model.CoverageStatus;
 import com.naver.nid.cover.parser.coverage.model.FileCoverageReport;
 import com.naver.nid.cover.parser.coverage.model.LineCoverageReport;
+import com.naver.nid.cover.util.PathUtils;
 import lombok.extern.slf4j.Slf4j;
 import org.xml.sax.Attributes;
 
 import java.math.BigInteger;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -46,7 +49,8 @@ public class CoberturaCoverageReportHandler extends CoverageReportXmlHandler {
 
 	private boolean isNowMethod; // 현재 메소드 내부인지
 
-	private Map<String, FileCoverageReport> reports = new HashMap<>();
+	private final Map<Path, FileCoverageReport> reports = new HashMap<>();
+
 	private List<LineCoverageReport> lineReports;
 	private FileCoverageReport current;
 
@@ -107,18 +111,18 @@ private CoverageStatus getCoverageStatus(Attributes attributes) {
 	 * @param attributes xml attribute
 	 */
 	private void initClass(Attributes attributes) {
-		String fileName = attributes.getValue(ATTR_FILENAME);
+		Path filePath = Paths.get(PathUtils.generalizeSeparator(attributes.getValue(ATTR_FILENAME)));
 		if(log.isDebugEnabled()) {
-			log.debug("parse class {}({})", attributes.getValue("name"), fileName);
+			log.debug("parse class {}({})", attributes.getValue("name"), filePath);
 		}
-		if (reports.containsKey(fileName)) {
-			current = reports.get(fileName);
+		if (reports.containsKey(filePath)) {
+			current = reports.get(filePath);
 			lineReports = current.getLineCoverageReportList();
 		} else {
 			current = new FileCoverageReport();
 			lineReports = new ArrayList<>();
-
-			current.setFileName(fileName);
+			String fileName = filePath.getFileName().toString();
+			current.setFileName(filePath);
 			current.setType(fileName.substring(fileName.lastIndexOf('.') + 1));
 			current.setLineCoverageReportList(lineReports);
 		}

cover-checker-cobertura/src/test/java/com/naver/nid/cover/cobertura/CoberturaXmlReportParserTest.java

@@ -20,7 +20,7 @@ public void parseCobertura() {
 		List<FileCoverageReport> parsed = parser.parse(getClass().getClassLoader().getResource("reports/coverage.xml"));
 		assertEquals(1, parsed.size());
 		assertSame(CoverageStatus.COVERED, parsed.stream()
-				.filter(r -> r.getFileName().contains("CoberturaReportHandler"))
+				.filter(r -> r.getFileName().endsWith("CoberturaReportHandler.java"))
 				.findFirst().flatMap(r -> r.getLineCoverageReportList().stream()
 						.filter(lr -> lr.getLineNum() == 68).findFirst()).orElseThrow(AssertionError::new).getStatus());
 

cover-checker-console/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <artifactId>coverchecker</artifactId>
         <groupId>com.naver.nid</groupId>
-        <version>1.4.1</version>
+        <version>1.4.2</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
@@ -81,7 +81,7 @@
                             </addDefaultImplementationEntries>
                         </manifest>
                     </archive>
-                    <finalName>${artifactId}</finalName>
+                    <finalName>${project.artifactId}</finalName>
                 </configuration>
             </plugin>
         </plugins>

cover-checker-core/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <artifactId>coverchecker</artifactId>
         <groupId>com.naver.nid</groupId>
-        <version>1.4.1</version>
+        <version>1.4.2</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

cover-checker-core/src/main/java/com/naver/nid/cover/checker/NewCoverageChecker.java

@@ -26,6 +26,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.nio.file.Path;
 import java.util.*;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -50,7 +51,7 @@ public class NewCoverageChecker {
 	 * @return 전체 커버리지, 파일 별 커버리지
 	 */
 	public NewCoverageCheckReport check(List<FileCoverageReport> coverage, List<Diff> diff, int threshold, int fileThreshold) {
-		Map<String, List<Line>> diffMap = diff.stream()
+		Map<Path, List<Line>> diffMap = diff.stream()
 				.filter(Objects::nonNull)
 				.peek(d -> logger.debug("diff file {}", d.getFileName()))
 				.filter(d -> !d.getFileName().startsWith("src/test"))
@@ -63,7 +64,7 @@ public NewCoverageCheckReport check(List<FileCoverageReport> coverage, List<Diff
 								.collect(Collectors.toList())
 						, (u1, u2) -> Stream.concat(u1.stream(), u2.stream()).collect(Collectors.toList())));
 
-		Map<String, List<LineCoverageReport>> coverageMap = coverage.stream()
+		Map<Path, List<LineCoverageReport>> coverageMap = coverage.stream()
 				.peek(r -> logger.debug("file coverage {}", r.getFileName()))
 				.collect(Collectors.toMap(FileCoverageReport::getFileName
 						, FileCoverageReport::getLineCoverageReportList
@@ -84,14 +85,14 @@ public NewCoverageCheckReport check(List<FileCoverageReport> coverage, List<Diff
 	 * @param newCodeLines   add line of code for each files
 	 * @return new line of code coverage result
 	 */
-	private NewCoverageCheckReport combine(Map<String, List<LineCoverageReport>> coverageReport, Map<String, List<Line>> newCodeLines) {
+	private NewCoverageCheckReport combine(Map<Path, List<LineCoverageReport>> coverageReport, Map<Path, List<Line>> newCodeLines) {
 		int totalAddLineCount = 0;
 		int coveredLineCount = 0;
 
-		Set<String> files = new HashSet<>(coverageReport.keySet());
+		Set<Path> files = new HashSet<>(coverageReport.keySet());
 
 		List<NewCoveredFile> coveredFileList = new ArrayList<>();
-		for (String file : files) {
+		for (Path file : files) {
 			// TODO 다른 모듈의 동일 패키지 동일 파일 이름일 경우에 대한 처리 필요
 
 			// 코드 커버리지의 끝 경로가 같은 경우에 대해 검색

cover-checker-core/src/main/java/com/naver/nid/cover/checker/model/NewCoveredFile.java

@@ -18,14 +18,16 @@
 import lombok.Builder;
 import lombok.Data;
 
+import java.nio.file.Path;
+
 import static com.naver.nid.cover.checker.model.ResultIcon.CHECK_FILE_FAIL;
 import static com.naver.nid.cover.checker.model.ResultIcon.CHECK_FILE_PASS;
 
 @Data
 @Builder
 public class NewCoveredFile {
 
-	private String name;
+	private Path name;
 	private int addedLine;
 	private int addedCoverLine;
 	private int threshold;

cover-checker-core/src/main/java/com/naver/nid/cover/parser/coverage/model/FileCoverageReport.java

@@ -19,14 +19,15 @@
 import lombok.Data;
 import lombok.NoArgsConstructor;
 
+import java.nio.file.Path;
 import java.util.List;
 import java.util.stream.Collectors;
 
 @Data
 @AllArgsConstructor
 @NoArgsConstructor
 public class FileCoverageReport {
-	private String fileName;
+	private Path fileName;
 	private String type;
 	private List<LineCoverageReport> lineCoverageReportList;
 

cover-checker-core/src/main/java/com/naver/nid/cover/parser/diff/DiffMapper.java

@@ -17,8 +17,11 @@
 
 import com.naver.nid.cover.parser.diff.exception.ParseException;
 import com.naver.nid.cover.parser.diff.model.*;
+import com.naver.nid.cover.util.PathUtils;
 import lombok.extern.slf4j.Slf4j;
 
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -49,7 +52,7 @@ public Diff apply(RawDiff rawDiff) {
 		Diff diff = new Diff();
 
 		// set file name
-		Optional<String> fileName = Optional.ofNullable(rawDiff.getFileName()).map(this::removeNewFilePrefix);
+		Optional<Path> fileName = Optional.ofNullable(rawDiff.getFileName()).map(this::removeNewFilePrefix);
 		log.info("parse diff / {}", fileName);
 
 		if (!fileName.isPresent()) {
@@ -98,11 +101,11 @@ private boolean isEof(String line) {
 		return line.contains("\\ No newline at end of file");
 	}
 
-	private String removeNewFilePrefix(String path) {
+	private Path removeNewFilePrefix(String path) {
 		if (path.startsWith("b/")) {
 			path = path.replaceFirst("b/", "");
 		}
-		return path;
+		return Paths.get(PathUtils.generalizeSeparator(path));
 	}
 
 	private Optional<List<DiffSection>> getDiffSectionListFromRawDiff(RawDiff rawDiff) {

cover-checker-core/src/main/java/com/naver/nid/cover/parser/diff/model/Diff.java

@@ -17,6 +17,7 @@
 
 import lombok.*;
 
+import java.nio.file.Path;
 import java.util.List;
 
 /**
@@ -29,6 +30,6 @@
 @AllArgsConstructor
 @NoArgsConstructor
 public class Diff {
-	private String fileName;
+	private Path fileName;
 	private List<DiffSection> diffSectionList;
 }

cover-checker-core/src/main/java/com/naver/nid/cover/util/PathUtils.java

@@ -0,0 +1,46 @@
+/*
+	Copyright 2021 NAVER Corp.
+
+	Licensed under the Apache License, Version 2.0 (the "License");
+	you may not use this file except in compliance with the License.
+	You may obtain a copy of the License at
+
+		http://www.apache.org/licenses/LICENSE-2.0
+
+	Unless required by applicable law or agreed to in writing, software
+	distributed under the License is distributed on an "AS IS" BASIS,
+	WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	See the License for the specific language governing permissions and
+	limitations under the License.
+ */
+package com.naver.nid.cover.util;
+
+import java.io.File;
+import java.util.regex.Matcher;
+
+public class PathUtils {
+    private PathUtils() {
+    }
+
+    /**
+     * <p>generalize file path</p>
+     *
+     * <h3>in Windows</h3>
+     * <ul>
+     * <li>- test/test.java -> test\test.java</li>
+     * <li>- test\test.java -> test\test.java</li>
+     * </ul>
+     *
+     * <h3>in Posix</h3>
+     *  <ul>
+     *  <li>- test/test.java -> test/test.java</li>
+     *  <li>- test\test.java -> test/test.java</li>
+     *  </ul>
+     * @param path path from reports(github, jacoco, cobertura...)
+     * @return generalized path that replaced separator that OS used
+     */
+    public static String generalizeSeparator(String path) {
+        if (path == null || path.isEmpty()) return path;
+        return path.replaceAll("[\\\\/]", Matcher.quoteReplacement(File.separator));
+    }
+}

cover-checker-core/src/test/java/com/naver/nid/cover/CoverCheckerTest.java

@@ -21,6 +21,7 @@
 import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -54,8 +55,8 @@ public void testCheck() {
 				, Line.builder().lineNumber(2).type(ModifyType.ADD).build());
 
 		List<DiffSection> diffSectionList = Collections.singletonList(DiffSection.builder().lineList(lines).build());
-		Stream<Diff> diffStream = Stream.of(Diff.builder().fileName("test.java").diffSectionList(diffSectionList).build(),
-			Diff.builder().fileName("test2.java").diffSectionList(diffSectionList).build());
+		Stream<Diff> diffStream = Stream.of(Diff.builder().fileName(Paths.get("test.java")).diffSectionList(diffSectionList).build(),
+			Diff.builder().fileName(Paths.get("test2.java")).diffSectionList(diffSectionList).build());
 
 
 		LineCoverageReport lineCoverageReport = new LineCoverageReport();
@@ -68,11 +69,11 @@ public void testCheck() {
 
 		FileCoverageReport fileCoverageReport = new FileCoverageReport();
 		fileCoverageReport.setType("java");
-		fileCoverageReport.setFileName("test.java");
+		fileCoverageReport.setFileName(Paths.get("test.java"));
 		fileCoverageReport.setLineCoverageReportList(Arrays.asList(lineCoverageReport, lineCoverageReport2));
 		FileCoverageReport fileCoverageReport2 = new FileCoverageReport();
 		fileCoverageReport.setType("java");
-		fileCoverageReport.setFileName("test2.java");
+		fileCoverageReport.setFileName(Paths.get("test2.java"));
 		fileCoverageReport.setLineCoverageReportList(Arrays.asList(lineCoverageReport, lineCoverageReport2));
 		List<FileCoverageReport> coverageModule1 = Collections.singletonList(fileCoverageReport);
 		List<FileCoverageReport> coverageModule2 = Collections.singletonList(fileCoverageReport2);
@@ -83,7 +84,7 @@ public void testCheck() {
 				.coveredNewLine(1)
 				.coveredFilesInfo(
 						Collections.singletonList(NewCoveredFile.builder()
-								.name("test.java")
+								.name(Paths.get("test.java"))
 								.addedLine(2)
 								.addedCoverLine(1)
 								.build()))