Skip to content

Commit 39a9fa5

Browse files
mawinter69mawinter69
committed
fix runs are added to all nodes on completion
the updateResult was wrongly adding a run to the list when there was no entry for the run. But here adding the run is wrong as we should only update the result. When the run is not found it means the run was not happening on that node. Also write the file only when it has really changed
1 parent 6ad3482 commit 39a9fa5

File tree

2 files changed

+48
-28
lines changed

2 files changed

+48
-28
lines changed

‎src/main/java/io/jenkins/plugins/agent_build_history/BuildHistoryFileManager.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
import java.io.FileInputStream;
1010
import java.io.FileNotFoundException;
1111
import java.io.FileOutputStream;
12+
import java.io.FileWriter;
1213
import java.io.IOException;
1314
import java.io.InputStreamReader;
1415
import java.io.OutputStreamWriter;
16+
import java.io.StringWriter;
1517
import java.nio.charset.StandardCharsets;
1618
import java.util.ArrayList;
1719
import java.util.Collections;
@@ -61,27 +63,31 @@ public static List<String> readIndexFile(String nodeName, String storageDir) {
6163

6264
public static void updateResult(String nodeName, Run<?, ?> run, String storageDir) {
6365
Object lock = getNodeLock(nodeName);
66+
String runIdentifier = run.getParent().getFullName() + separator + run.getNumber() + separator;
6467
synchronized (lock) {
6568
List<String> indexLines = readIndexFile(nodeName, storageDir);
66-
File indexFile = new File(storageDir + "/" + nodeName + "_index.txt");
67-
try (BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(indexFile), StandardCharsets.UTF_8))) {
69+
StringWriter buffer = new StringWriter();
70+
try (BufferedWriter writer = new BufferedWriter(buffer)) {
6871
boolean found = false;
6972
for (String line : indexLines) {
70-
if (line.startsWith(run.getParent().getFullName() + separator + run.getNumber() + separator)) {
73+
if (line.startsWith(runIdentifier)) {
7174
if (!line.endsWith(separator + run.getResult())) {
7275
writeLine(writer, run);
76+
found = true;
7377
} else {
7478
writer.write(line);
7579
}
76-
found = true;
7780
} else {
7881
writer.write(line);
7982
}
8083
writer.newLine();
8184
}
82-
if (!found) {
83-
writeLine(writer, run);
84-
writer.newLine();
85+
if (found) {
86+
writer.flush();
87+
File indexFile = new File(storageDir + "/" + nodeName + "_index.txt");
88+
try (FileWriter fileWriter = new FileWriter(indexFile, StandardCharsets.UTF_8)) {
89+
fileWriter.write(buffer.toString());
90+
}
8591
}
8692
} catch (IOException e) {
8793
LOGGER.log(Level.WARNING, "Failed to update result in index-file for node " + nodeName, e);
@@ -137,12 +143,13 @@ public static Set<String> getAllSavedNodeNames(String storageDir) {
137143

138144
public static void deleteExecution(String nodeName, String jobName, int buildNumber, String storageDir) {
139145
Object lock = getNodeLock(nodeName);
146+
String runIdentifier = jobName + separator + buildNumber + separator;
140147
synchronized (lock) {
141148
List<String> indexLines = readIndexFile(nodeName, storageDir);
142149
File indexFile = new File(storageDir + "/" + nodeName + "_index.txt");
143150
try (BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(indexFile), StandardCharsets.UTF_8))) {
144151
for (String line : indexLines) {
145-
if (!line.startsWith(jobName + separator + buildNumber + separator)) {
152+
if (!line.startsWith(runIdentifier)) {
146153
writer.write(line);
147154
writer.newLine();
148155
}

‎src/test/java/io/jenkins/plugins/agent_build_history/BuildHistoryFileManagerTest.java‎

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44
import static org.junit.jupiter.api.Assertions.assertFalse;
55
import static org.junit.jupiter.api.Assertions.assertTrue;
66

7-
import hudson.model.FreeStyleBuild;
87
import hudson.model.FreeStyleProject;
98
import hudson.model.Run;
10-
import hudson.model.queue.QueueTaskFuture;
119
import java.io.File;
1210
import java.util.List;
1311
import java.util.Set;
@@ -33,7 +31,8 @@ void setUp(JenkinsRule rule) {
3331

3432
@Test
3533
void testUpdateResult() throws Exception {
36-
String nodeName = "test-node";
34+
String nodeName1 = "test-node-1";
35+
String nodeName2 = "test-node-2";
3736
String jobName = "test-job";
3837
jenkinsRule.jenkins.setNumExecutors(2);
3938

@@ -46,30 +45,44 @@ void testUpdateResult() throws Exception {
4645
Run<?, ?> run2 = project.scheduleBuild2(0).waitForStart();
4746

4847
// Simulate adding a run to the node index
49-
BuildHistoryFileManager.addRunToNodeIndex(nodeName, run1, storageDir.getAbsolutePath());
50-
BuildHistoryFileManager.addRunToNodeIndex(nodeName, run2, storageDir.getAbsolutePath());
51-
52-
List<String> indexEntries = BuildHistoryFileManager.readIndexFile(nodeName, storageDir.getAbsolutePath());
53-
assertEquals(2, indexEntries.size());
54-
assertTrue(indexEntries.get(0).contains(jobName+";1;"), "Index should contain build number");
55-
assertFalse(indexEntries.get(0).contains("SUCCESS"), "Index should not contain the updated result");
56-
assertTrue(indexEntries.get(1).contains(jobName+";2;"), "Index should contain build number");
57-
assertFalse(indexEntries.get(1).contains("SUCCESS"), "Index should not contain the updated result");
48+
BuildHistoryFileManager.addRunToNodeIndex(nodeName1, run1, storageDir.getAbsolutePath());
49+
BuildHistoryFileManager.addRunToNodeIndex(nodeName2, run2, storageDir.getAbsolutePath());
50+
51+
List<String> indexEntries1 = BuildHistoryFileManager.readIndexFile(nodeName1, storageDir.getAbsolutePath());
52+
List<String> indexEntries2 = BuildHistoryFileManager.readIndexFile(nodeName2, storageDir.getAbsolutePath());
53+
assertEquals(1, indexEntries1.size());
54+
assertEquals(1, indexEntries2.size());
55+
assertTrue(indexEntries1.get(0).contains(jobName+";1;"), "Index should contain build number");
56+
assertFalse(indexEntries1.get(0).contains("SUCCESS"), "Index should not contain the updated result");
57+
assertTrue(indexEntries2.get(0).contains(jobName+";2;"), "Index should contain build number");
58+
assertFalse(indexEntries2.get(0).contains("SUCCESS"), "Index should not contain the updated result");
5859

5960
jenkinsRule.waitForCompletion(run1);
6061
jenkinsRule.waitForCompletion(run2);
6162

6263
// Update the result of the run
63-
BuildHistoryFileManager.updateResult(nodeName, run1, storageDir.getAbsolutePath());
64-
BuildHistoryFileManager.updateResult(nodeName, run2, storageDir.getAbsolutePath());
64+
BuildHistoryFileManager.updateResult(nodeName1, run1, storageDir.getAbsolutePath());
65+
BuildHistoryFileManager.updateResult(nodeName2, run2, storageDir.getAbsolutePath());
6566

6667
// Verify that the index file contains the updated result
67-
indexEntries = BuildHistoryFileManager.readIndexFile(nodeName, storageDir.getAbsolutePath());
68-
assertEquals(2, indexEntries.size());
69-
assertTrue(indexEntries.get(0).contains(jobName+";1;"), "Index should contain build number");
70-
assertTrue(indexEntries.get(0).contains("SUCCESS"), "Index should contain the updated result");
71-
assertTrue(indexEntries.get(1).contains(jobName+";2;"), "Index should contain build number");
72-
assertTrue(indexEntries.get(1).contains("SUCCESS"), "Index should contain the updated result");
68+
indexEntries1 = BuildHistoryFileManager.readIndexFile(nodeName1, storageDir.getAbsolutePath());
69+
indexEntries2 = BuildHistoryFileManager.readIndexFile(nodeName2, storageDir.getAbsolutePath());
70+
assertEquals(1, indexEntries1.size());
71+
assertEquals(1, indexEntries2.size());
72+
assertTrue(indexEntries1.get(0).contains(jobName+";1;"), "Index should contain build number");
73+
assertTrue(indexEntries1.get(0).contains("SUCCESS"), "Index should contain the updated result");
74+
assertTrue(indexEntries2.get(0).contains(jobName+";2;"), "Index should contain build number");
75+
assertTrue(indexEntries2.get(0).contains("SUCCESS"), "Index should contain the updated result");
76+
77+
// Verify that the index file is not updated when the run was not on that node
78+
BuildHistoryFileManager.updateResult(nodeName2, run1, storageDir.getAbsolutePath());
79+
BuildHistoryFileManager.updateResult(nodeName1, run2, storageDir.getAbsolutePath());
80+
assertEquals(1, indexEntries1.size());
81+
assertEquals(1, indexEntries2.size());
82+
assertTrue(indexEntries1.get(0).contains(jobName+";1;"), "Index should contain build number");
83+
assertTrue(indexEntries1.get(0).contains("SUCCESS"), "Index should contain the updated result");
84+
assertTrue(indexEntries2.get(0).contains(jobName+";2;"), "Index should contain build number");
85+
assertTrue(indexEntries2.get(0).contains("SUCCESS"), "Index should contain the updated result");
7386
}
7487

7588
@Test

0 commit comments

Comments
 (0)