From 29cc46ba20debc188f5f3e32b160e17ecbc360d3 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Sun, 8 Dec 2024 17:22:42 +0100 Subject: [PATCH 1/6] add an option to ignore whitespace during diffs --- .../service/CommitHistoryService.java | 2 +- ...ogrammingExerciseGitDiffReportService.java | 22 ++-- .../web/GitDiffReportParserService.java | 108 ++++++++++++------ .../artemis/{ => atlas}/UnionFindTest.java | 2 +- ...gExerciseGitDiffReportIntegrationTest.java | 26 ++++- ...mmingExerciseGitDiffReportServiceTest.java | 9 ++ 6 files changed, 115 insertions(+), 54 deletions(-) rename src/test/java/de/tum/cit/aet/artemis/{ => atlas}/UnionFindTest.java (98%) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/CommitHistoryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/CommitHistoryService.java index 3d8beec05006..9ee663c55c96 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/CommitHistoryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/CommitHistoryService.java @@ -107,7 +107,7 @@ private ProgrammingExerciseGitDiffReport createReport(Repository repository, Rev diffs.append(out.toString(StandardCharsets.UTF_8)); } - var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diffs.toString(), false); + var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diffs.toString(), false, false); var report = new ProgrammingExerciseGitDiffReport(); for (ProgrammingExerciseGitDiffEntry gitDiffEntry : programmingExerciseGitDiffEntries) { gitDiffEntry.setGitDiffReport(report); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseGitDiffReportService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseGitDiffReportService.java index 7ace2505cf1c..6950e1216df5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseGitDiffReportService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseGitDiffReportService.java @@ -109,7 +109,7 @@ public ProgrammingExerciseGitDiffReport updateReport(ProgrammingExercise program var templateHash = templateSubmission.getCommitHash(); var solutionHash = solutionSubmission.getCommitHash(); - var existingReport = this.getReportOfExercise(programmingExercise); + var existingReport = getReportOfExercise(programmingExercise); if (existingReport != null && canUseExistingReport(existingReport, templateHash, solutionHash)) { return existingReport; } @@ -164,7 +164,7 @@ else if (reports.size() == 1) { * @return The report or null if none can be generated */ public ProgrammingExerciseGitDiffReport getOrCreateReportOfExercise(ProgrammingExercise programmingExercise) { - var report = this.getReportOfExercise(programmingExercise); + var report = getReportOfExercise(programmingExercise); if (report == null) { return updateReport(programmingExercise); } @@ -215,7 +215,7 @@ public int calculateNumberOfDiffLinesBetweenRepos(VcsRepositoryUri urlRepoA, Pat try (var diffOutputStream = new ByteArrayOutputStream(); var git = Git.wrap(repoB)) { git.diff().setOldTree(treeParserRepoB).setNewTree(treeParserRepoA).setOutputStream(diffOutputStream).call(); var diff = diffOutputStream.toString(); - return gitDiffReportParserService.extractDiffEntries(diff, true).stream().mapToInt(ProgrammingExerciseGitDiffEntry::getLineCount).sum(); + return gitDiffReportParserService.extractDiffEntries(diff, true, false).stream().mapToInt(ProgrammingExerciseGitDiffEntry::getLineCount).sum(); } catch (IOException | GitAPIException e) { log.error("Error calculating number of diff lines between repositories: urlRepoA={}, urlRepoB={}.", urlRepoA, urlRepoB, e); @@ -234,6 +234,7 @@ public int calculateNumberOfDiffLinesBetweenRepos(VcsRepositoryUri urlRepoA, Pat */ private ProgrammingExerciseGitDiffReport generateReport(TemplateProgrammingExerciseParticipation templateParticipation, SolutionProgrammingExerciseParticipation solutionParticipation) throws GitAPIException, IOException { + // TODO: in case of LocalVC, we should calculate the diff in the bare origin repository Repository templateRepo = prepareTemplateRepository(templateParticipation); var solutionRepo = gitService.getOrCheckoutRepository(solutionParticipation.getVcsRepositoryUri(), true); gitService.resetToOriginHead(solutionRepo); @@ -306,19 +307,20 @@ private ProgrammingExerciseGitDiffReport parseFilesAndCreateReport(Repository re * It parses all files of the repositories in their directories on the file system and creates a report containing the changes. * Both repositories have to be checked out at the commit that should be compared and be in different directories * - * @param repo1 The first repository - * @param oldTreeParser The tree parser for the first repository - * @param newTreeParser The tree parser for the second repository + * @param firstRepo The first repository + * @param firstRepoTreeParser The tree parser for the first repository + * @param secondRepoTreeParser The tree parser for the second repository * @return The report with the changes between the two repositories at their checked out state * @throws IOException If an error occurs while accessing the file system * @throws GitAPIException If an error occurs while accessing the git repository */ @NotNull - private ProgrammingExerciseGitDiffReport createReport(Repository repo1, FileTreeIterator oldTreeParser, FileTreeIterator newTreeParser) throws IOException, GitAPIException { - try (ByteArrayOutputStream diffOutputStream = new ByteArrayOutputStream(); Git git = Git.wrap(repo1)) { - git.diff().setOldTree(oldTreeParser).setNewTree(newTreeParser).setOutputStream(diffOutputStream).call(); + private ProgrammingExerciseGitDiffReport createReport(Repository firstRepo, FileTreeIterator firstRepoTreeParser, FileTreeIterator secondRepoTreeParser) + throws IOException, GitAPIException { + try (ByteArrayOutputStream diffOutputStream = new ByteArrayOutputStream(); Git git = Git.wrap(firstRepo)) { + git.diff().setOldTree(firstRepoTreeParser).setNewTree(secondRepoTreeParser).setOutputStream(diffOutputStream).call(); var diff = diffOutputStream.toString(); - var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diff, false); + var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diff, false, true); var report = new ProgrammingExerciseGitDiffReport(); for (ProgrammingExerciseGitDiffEntry gitDiffEntry : programmingExerciseGitDiffEntries) { gitDiffEntry.setGitDiffReport(report); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java index a12dff5575d6..7c5af07893e5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java @@ -32,7 +32,7 @@ public class GitDiffReportParserService { * @param useAbsoluteLineCount Whether to use absolute line count or previous line count * @return The extracted ProgrammingExerciseGitDiffEntries */ - public List extractDiffEntries(String diff, boolean useAbsoluteLineCount) { + public List extractDiffEntries(String diff, boolean useAbsoluteLineCount, boolean ignoreWhitespace) { var lines = diff.split("\n"); var parserState = new ParserState(); Map renamedFilePaths = new HashMap<>(); @@ -44,8 +44,7 @@ public List extractDiffEntries(String diff, boo continue; } - // Files may be renamed without changes, in which case the lineMatcher will never match the entry - // We store this information separately so it is not lost + // Check for renamed files if (line.startsWith(PREFIX_RENAME_FROM) && i + 1 < lines.length) { var nextLine = lines[i + 1]; if (nextLine.startsWith(PREFIX_RENAME_TO)) { @@ -57,34 +56,35 @@ public List extractDiffEntries(String diff, boo var lineMatcher = gitDiffLinePattern.matcher(line); if (lineMatcher.matches()) { - handleNewDiffBlock(lines, i, parserState, lineMatcher); + handleNewDiffBlock(lines, i, parserState, lineMatcher, ignoreWhitespace); } - else if (!parserState.deactivateCodeReading) { + else if (!parserState.deactivateCodeReading && !line.isEmpty()) { switch (line.charAt(0)) { - case '+' -> handleAddition(parserState); - case '-' -> handleRemoval(parserState, useAbsoluteLineCount); - case ' ' -> handleUnchanged(parserState); + case '+' -> handleAddition(parserState, line); + case '-' -> handleRemoval(parserState, useAbsoluteLineCount, line); + case ' ' -> handleUnchanged(parserState, ignoreWhitespace); default -> parserState.deactivateCodeReading = true; } } } - if (!parserState.currentEntry.isEmpty()) { - parserState.entries.add(parserState.currentEntry); - } - // Add an empty diff entry for renamed files without changes + + // Check the last entry + finalizeEntry(parserState, ignoreWhitespace); + + // Add empty entries for renamed files without changes for (var entry : renamedFilePaths.entrySet()) { var diffEntry = new ProgrammingExerciseGitDiffEntry(); diffEntry.setFilePath(entry.getValue()); diffEntry.setPreviousFilePath(entry.getKey()); parserState.entries.add(diffEntry); } + return parserState.entries; } - private void handleNewDiffBlock(String[] lines, int currentLine, ParserState parserState, Matcher lineMatcher) { - if (!parserState.currentEntry.isEmpty()) { - parserState.entries.add(parserState.currentEntry); - } + private void handleNewDiffBlock(String[] lines, int currentLine, ParserState parserState, Matcher lineMatcher, boolean ignoreWhitespace) { + finalizeEntry(parserState, ignoreWhitespace); + // Start of a new file var newFilePath = getFilePath(lines, currentLine); var newPreviousFilePath = getPreviousFilePath(lines, currentLine); @@ -92,40 +92,45 @@ private void handleNewDiffBlock(String[] lines, int currentLine, ParserState par parserState.currentFilePath = newFilePath; parserState.currentPreviousFilePath = newPreviousFilePath; } + parserState.currentEntry = new ProgrammingExerciseGitDiffEntry(); parserState.currentEntry.setFilePath(parserState.currentFilePath); parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath); parserState.currentLineCount = Integer.parseInt(lineMatcher.group("newLine")); parserState.currentPreviousLineCount = Integer.parseInt(lineMatcher.group("previousLine")); parserState.deactivateCodeReading = false; + parserState.addedLines.clear(); + parserState.removedLines.clear(); } - private void handleUnchanged(ParserState parserState) { - var entry = parserState.currentEntry; - if (!entry.isEmpty()) { - parserState.entries.add(entry); - } - entry = new ProgrammingExerciseGitDiffEntry(); - entry.setFilePath(parserState.currentFilePath); - entry.setPreviousFilePath(parserState.currentPreviousFilePath); + private void handleUnchanged(ParserState parserState, boolean ignoreWhitespace) { + finalizeEntry(parserState, ignoreWhitespace); + parserState.currentEntry = new ProgrammingExerciseGitDiffEntry(); + parserState.currentEntry.setFilePath(parserState.currentFilePath); + parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath); - parserState.currentEntry = entry; parserState.lastLineRemoveOperation = false; parserState.currentLineCount++; parserState.currentPreviousLineCount++; + parserState.addedLines.clear(); + parserState.removedLines.clear(); } - private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount) { + private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount, String line) { var entry = parserState.currentEntry; if (!parserState.lastLineRemoveOperation && !entry.isEmpty()) { - parserState.entries.add(entry); - entry = new ProgrammingExerciseGitDiffEntry(); - entry.setFilePath(parserState.currentFilePath); - entry.setPreviousFilePath(parserState.currentPreviousFilePath); + finalizeEntry(parserState, false); + parserState.currentEntry = new ProgrammingExerciseGitDiffEntry(); + parserState.currentEntry.setFilePath(parserState.currentFilePath); + parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath); } - if (entry.getPreviousLineCount() == null) { - entry.setPreviousLineCount(0); - entry.setPreviousStartLine(parserState.currentPreviousLineCount); + + // Store removed line + parserState.removedLines.add(line.substring(1)); + + if (parserState.currentEntry.getPreviousLineCount() == null) { + parserState.currentEntry.setPreviousLineCount(0); + parserState.currentEntry.setPreviousStartLine(parserState.currentPreviousLineCount); } if (useAbsoluteLineCount) { if (parserState.currentEntry.getLineCount() == null) { @@ -135,15 +140,17 @@ private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount parserState.currentEntry.setLineCount(parserState.currentEntry.getLineCount() + 1); } else { - entry.setPreviousLineCount(entry.getPreviousLineCount() + 1); + parserState.currentEntry.setPreviousLineCount(parserState.currentEntry.getPreviousLineCount() + 1); } - parserState.currentEntry = entry; parserState.lastLineRemoveOperation = true; parserState.currentPreviousLineCount++; } - private void handleAddition(ParserState parserState) { + private void handleAddition(ParserState parserState, String line) { + // Store added line + parserState.addedLines.add(line.substring(1)); + if (parserState.currentEntry.getLineCount() == null) { parserState.currentEntry.setLineCount(0); parserState.currentEntry.setStartLine(parserState.currentLineCount); @@ -154,6 +161,29 @@ private void handleAddition(ParserState parserState) { parserState.currentLineCount++; } + private void finalizeEntry(ParserState parserState, boolean ignoreWhitespace) { + if (!parserState.currentEntry.isEmpty()) { + if (!ignoreWhitespace || !isWhitespaceOnlyChange(parserState.addedLines, parserState.removedLines)) { + parserState.entries.add(parserState.currentEntry); + } + } + } + + private boolean isWhitespaceOnlyChange(List addedLines, List removedLines) { + if (addedLines.size() != removedLines.size()) { + return false; // Different number of lines changed, definitely not whitespace only + } + + for (int i = 0; i < addedLines.size(); i++) { + String added = addedLines.get(i).replaceAll("\\s+", ""); + String removed = removedLines.get(i).replaceAll("\\s+", ""); + if (!added.equals(removed)) { + return false; + } + } + return true; + } + /** * Extracts the file path from the raw git-diff for a specified diff block * @@ -219,6 +249,10 @@ private static class ParserState { private int currentPreviousLineCount; + private final List addedLines; + + private final List removedLines; + public ParserState() { entries = new ArrayList<>(); currentEntry = new ProgrammingExerciseGitDiffEntry(); @@ -226,6 +260,8 @@ public ParserState() { lastLineRemoveOperation = false; currentLineCount = 0; currentPreviousLineCount = 0; + addedLines = new ArrayList<>(); + removedLines = new ArrayList<>(); } } } diff --git a/src/test/java/de/tum/cit/aet/artemis/UnionFindTest.java b/src/test/java/de/tum/cit/aet/artemis/atlas/UnionFindTest.java similarity index 98% rename from src/test/java/de/tum/cit/aet/artemis/UnionFindTest.java rename to src/test/java/de/tum/cit/aet/artemis/atlas/UnionFindTest.java index 4d6b0b8ebbc6..5709a598a27c 100644 --- a/src/test/java/de/tum/cit/aet/artemis/UnionFindTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/atlas/UnionFindTest.java @@ -1,4 +1,4 @@ -package de.tum.cit.aet.artemis; +package de.tum.cit.aet.artemis.atlas; import static org.assertj.core.api.Assertions.assertThat; diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java index df54e4dc10f5..f9c7f244d61d 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java @@ -70,7 +70,9 @@ void getGitDiffAsATutor() throws Exception { exercise = hestiaUtilTestService.setupTemplate(FILE_NAME, "TEST", exercise, templateRepo); exercise = hestiaUtilTestService.setupSolution(FILE_NAME, "TEST", exercise, solutionRepo); reportService.updateReport(exercise); - request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).isNull(); } @Test @@ -79,7 +81,9 @@ void getGitDiffAsAnEditor() throws Exception { exercise = hestiaUtilTestService.setupTemplate(FILE_NAME, "TEST", exercise, templateRepo); exercise = hestiaUtilTestService.setupSolution(FILE_NAME, "TEST", exercise, solutionRepo); reportService.updateReport(exercise); - request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).isNull(); } @Test @@ -88,7 +92,9 @@ void getGitDiffAsAnInstructor() throws Exception { exercise = hestiaUtilTestService.setupTemplate(FILE_NAME, "TEST", exercise, templateRepo); exercise = hestiaUtilTestService.setupSolution(FILE_NAME, "TEST", exercise, solutionRepo); reportService.updateReport(exercise); - request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/diff-report", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).isNull(); } @Test @@ -98,8 +104,10 @@ void getGitDiffBetweenTemplateAndSubmission() throws Exception { participationRepo.configureRepos("participationLocalRepo", "participationOriginRepo"); var studentLogin = TEST_PREFIX + "student1"; var submission = hestiaUtilTestService.setupSubmission(FILE_NAME, "TEST", exercise, participationRepo, studentLogin); - request.get("/api/programming-exercises/" + exercise.getId() + "/submissions/" + submission.getId() + "/diff-report-with-template", HttpStatus.OK, + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/submissions/" + submission.getId() + "/diff-report-with-template", HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).isNull(); } @Test @@ -121,8 +129,11 @@ void getGitDiffReportForCommits() throws Exception { var studentLogin = TEST_PREFIX + "student1"; var submission = hestiaUtilTestService.setupSubmission(FILE_NAME, "TEST", exercise, participationRepo, studentLogin); var submission2 = hestiaUtilTestService.setupSubmission(FILE_NAME, "TEST2", exercise, participationRepo, studentLogin); - request.get("/api/programming-exercises/" + exercise.getId() + "/commits/" + submission.getCommitHash() + "/diff-report/" + submission2.getCommitHash() + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/commits/" + submission.getCommitHash() + "/diff-report/" + submission2.getCommitHash() + "?participationId=" + submission.getParticipation().getId(), HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).hasSize(1); + // TODO: add more assertions } @Test @@ -179,8 +190,11 @@ void getGitDiffBetweenTwoSubmissions() throws Exception { var studentLogin = TEST_PREFIX + "student1"; var submission = hestiaUtilTestService.setupSubmission(FILE_NAME, "TEST", exercise, participationRepo, studentLogin); var submission2 = hestiaUtilTestService.setupSubmission(FILE_NAME, "TEST2", exercise, participationRepo, studentLogin); - request.get("/api/programming-exercises/" + exercise.getId() + "/submissions/" + submission.getId() + "/diff-report/" + submission2.getId(), HttpStatus.OK, + var report = request.get("/api/programming-exercises/" + exercise.getId() + "/submissions/" + submission.getId() + "/diff-report/" + submission2.getId(), HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); + assertThat(report).isNotNull(); + assertThat(report.getEntries()).hasSize(1); + // TODO: add more assertions } @Test diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportServiceTest.java index 2516a2f416fc..a79c8880e1fb 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportServiceTest.java @@ -136,6 +136,15 @@ void updateGitDiffDoubleModify() throws Exception { assertThat(entries.get(1).getLineCount()).isEqualTo(1); } + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void gitDiffWhitespace() throws Exception { + exercise = hestiaUtilTestService.setupTemplate(FILE_NAME, " ", exercise, templateRepo); + exercise = hestiaUtilTestService.setupSolution(FILE_NAME, "\t", exercise, solutionRepo); + var report = reportService.updateReport(exercise); + assertThat(report.getEntries()).hasSize(0); + } + @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void updateGitDiffReuseExisting() throws Exception { From b137ea9132b75612b8c63ea40d20a811612751ae Mon Sep 17 00:00:00 2001 From: Marcel Gaupp Date: Sun, 8 Dec 2024 20:40:26 +0100 Subject: [PATCH 2/6] Allow hunk headers --- .../aet/artemis/programming/web/GitDiffReportParserService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java index 7c5af07893e5..bb7e699be7c5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java @@ -23,7 +23,7 @@ public class GitDiffReportParserService { private static final String PREFIX_RENAME_TO = "rename to "; - private final Pattern gitDiffLinePattern = Pattern.compile("@@ -(?\\d+)(,(?\\d+))? \\+(?\\d+)(,(?\\d+))? @@"); + private final Pattern gitDiffLinePattern = Pattern.compile("@@ -(?\\d+)(,(?\\d+))? \\+(?\\d+)(,(?\\d+))? @@.*"); /** * Extracts the ProgrammingExerciseGitDiffEntry from the raw git-diff output From 7989b41d76188008be86c2eab5935ef63ce937b2 Mon Sep 17 00:00:00 2001 From: Marcel Gaupp Date: Sun, 8 Dec 2024 20:52:05 +0100 Subject: [PATCH 3/6] Only ignore trimmed whitespace --- .../artemis/programming/web/GitDiffReportParserService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java index bb7e699be7c5..c65f932d01a6 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java @@ -175,8 +175,8 @@ private boolean isWhitespaceOnlyChange(List addedLines, List rem } for (int i = 0; i < addedLines.size(); i++) { - String added = addedLines.get(i).replaceAll("\\s+", ""); - String removed = removedLines.get(i).replaceAll("\\s+", ""); + String added = addedLines.get(i).trim(); + String removed = removedLines.get(i).trim(); if (!added.equals(removed)) { return false; } From 234849c5a0667ccda43d74f149a47e25168da0d2 Mon Sep 17 00:00:00 2001 From: Marcel Gaupp Date: Tue, 10 Dec 2024 01:36:49 +0100 Subject: [PATCH 4/6] Add missing JavaDoc --- .../aet/artemis/programming/web/GitDiffReportParserService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java index c65f932d01a6..37042b8cdb06 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java @@ -30,6 +30,7 @@ public class GitDiffReportParserService { * * @param diff The raw git-diff output * @param useAbsoluteLineCount Whether to use absolute line count or previous line count + * @param ignoreWhitespace Whether to ignore entries where only leading and trailing whitespace differ * @return The extracted ProgrammingExerciseGitDiffEntries */ public List extractDiffEntries(String diff, boolean useAbsoluteLineCount, boolean ignoreWhitespace) { From fe91f754461076804cdf8069f6ca59ad605fe87c Mon Sep 17 00:00:00 2001 From: Marcel Gaupp Date: Tue, 10 Dec 2024 14:00:11 +0100 Subject: [PATCH 5/6] Add additional testing assertions --- ...mingExerciseGitDiffReportIntegrationTest.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java index f9c7f244d61d..453cb7ef80b7 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java @@ -133,7 +133,13 @@ void getGitDiffReportForCommits() throws Exception { + "?participationId=" + submission.getParticipation().getId(), HttpStatus.OK, ProgrammingExerciseGitDiffReport.class); assertThat(report).isNotNull(); assertThat(report.getEntries()).hasSize(1); - // TODO: add more assertions + var entry = report.getEntries().stream().findAny().orElseThrow(); + assertThat(entry.getPreviousFilePath()).isEqualTo(FILE_NAME); + assertThat(entry.getPreviousStartLine()).isEqualTo(1); + assertThat(entry.getPreviousLineCount()).isEqualTo(1); + assertThat(entry.getFilePath()).isEqualTo(FILE_NAME); + assertThat(entry.getStartLine()).isEqualTo(1); + assertThat(entry.getLineCount()).isEqualTo(1); } @Test @@ -194,7 +200,13 @@ void getGitDiffBetweenTwoSubmissions() throws Exception { ProgrammingExerciseGitDiffReport.class); assertThat(report).isNotNull(); assertThat(report.getEntries()).hasSize(1); - // TODO: add more assertions + var entry = report.getEntries().stream().findAny().orElseThrow(); + assertThat(entry.getPreviousFilePath()).isEqualTo(FILE_NAME); + assertThat(entry.getPreviousStartLine()).isEqualTo(1); + assertThat(entry.getPreviousLineCount()).isEqualTo(1); + assertThat(entry.getFilePath()).isEqualTo(FILE_NAME); + assertThat(entry.getStartLine()).isEqualTo(1); + assertThat(entry.getLineCount()).isEqualTo(1); } @Test From a248dc80636b217ef948966d500d3440e6128a8a Mon Sep 17 00:00:00 2001 From: Marcel Gaupp Date: Tue, 10 Dec 2024 14:10:06 +0100 Subject: [PATCH 6/6] Remove unused throws clause --- .../hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java index 453cb7ef80b7..b896af7ac582 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/ProgrammingExerciseGitDiffReportIntegrationTest.java @@ -38,7 +38,7 @@ class ProgrammingExerciseGitDiffReportIntegrationTest extends AbstractProgrammin private ProgrammingExercise exercise; @BeforeEach - void initTestCase() throws Exception { + void initTestCase() { Course course = courseUtilService.addEmptyCourse(); userUtilService.addUsers(TEST_PREFIX, 1, 1, 1, 1); exercise = ProgrammingExerciseFactory.generateProgrammingExercise(ZonedDateTime.now().minusDays(1), ZonedDateTime.now().plusDays(7), course);