Skip to content

Commit

Permalink
Merge pull request #493 from Pijukatel/JENKINS-66535
Browse files Browse the repository at this point in the history
[JENKINS-66535] Fix situation when failed builds are missing and only successful builds remain
  • Loading branch information
rsandell committed Jul 19, 2023
2 parents c8d57b5 + 8364ddf commit 39403ae
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 10 deletions.
Expand Up @@ -381,12 +381,15 @@ protected Integer getVerifiedValue(Result res, GerritTrigger trigger) {
/**
* Returns the minimum verified value for the build results in the memory.
* If no builds have contributed to verified value, this method returns null
* @param memoryImprint the memory.
* @param onlyBuilt only count builds that completed (no NOT_BUILT builds)
*
* @param memoryImprint the memory.
* @param onlyBuilt only count builds that completed (no NOT_BUILT builds)
* @param maxAllowedVerifiedValue Upper boundary on verified value.
* @return the lowest verified value.
*/
@CheckForNull
public Integer getMinimumVerifiedValue(MemoryImprint memoryImprint, boolean onlyBuilt) {
public Integer getMinimumVerifiedValue(MemoryImprint memoryImprint, boolean onlyBuilt,
Integer maxAllowedVerifiedValue) {
Integer verified = Integer.MAX_VALUE;
for (Entry entry : memoryImprint.getEntries()) {
if (entry == null) {
Expand Down Expand Up @@ -415,7 +418,7 @@ public Integer getMinimumVerifiedValue(MemoryImprint memoryImprint, boolean only
return null;
}

return verified;
return Math.min(verified, maxAllowedVerifiedValue);
}

/**
Expand Down Expand Up @@ -531,6 +534,7 @@ public String getBuildCompletedCommand(MemoryImprint memoryImprint, TaskListener
// builds were successful, unstable or failed, we find the minimum
// verified/code review value for the NOT_BUILT ones too.
boolean onlyCountBuilt = true;
Integer maxAllowedVerifiedValue = Integer.MAX_VALUE;
if (memoryImprint.wereAllBuildsSuccessful()) {
command = config.getGerritCmdBuildSuccessful();
} else if (memoryImprint.wereAnyBuildsFailed()) {
Expand All @@ -545,13 +549,17 @@ public String getBuildCompletedCommand(MemoryImprint memoryImprint, TaskListener
} else {
//Just as bad as failed for now.
command = config.getGerritCmdBuildFailed();
// Some builds could have failed, but are already deleted and not
// available for score calculation.
// Set pessimistic upper boundary on verified value.
maxAllowedVerifiedValue = config.getGerritBuildFailedVerifiedValue();
}

Integer verified = null;
Integer codeReview = null;
Notify notifyLevel = Notify.ALL;
if (memoryImprint.getEvent().isScorable()) {
verified = getMinimumVerifiedValue(memoryImprint, onlyCountBuilt);
verified = getMinimumVerifiedValue(memoryImprint, onlyCountBuilt, maxAllowedVerifiedValue);
codeReview = getMinimumCodeReviewValue(memoryImprint, onlyCountBuilt);
notifyLevel = getHighestNotificationLevel(memoryImprint, onlyCountBuilt);
}
Expand Down
Expand Up @@ -90,7 +90,7 @@ protected ReviewInput createReview() {
}
}
if (config.isRestVerified()) {
Integer verValue = parameterExpander.getMinimumVerifiedValue(memoryImprint, true);
Integer verValue = parameterExpander.getMinimumVerifiedValue(memoryImprint, true, Integer.MAX_VALUE);
if (verValue != null && verValue != Integer.MAX_VALUE) {
scoredLabels.add(new ReviewLabel(
LABEL_VERIFIED,
Expand Down
Expand Up @@ -103,14 +103,14 @@ public void testCodeReview() {
}

/**
* Tests that {@link ParameterExpander#getMinimumVerifiedValue(BuildMemory.MemoryImprint, boolean)}
* Tests that {@link ParameterExpander#getMinimumVerifiedValue(BuildMemory.MemoryImprint, boolean, Integer)}
* returns {@link TestParameter#expectedVerified}.
*/
@Test
public void testVerified() {
IGerritHudsonTriggerConfig config = Setup.createConfig();
ParameterExpander instance = new ParameterExpander(config);
Integer result = instance.getMinimumVerifiedValue(parameter.memoryImprint, true);
Integer result = instance.getMinimumVerifiedValue(parameter.memoryImprint, true, Integer.MAX_VALUE);
if (parameter.expectedVerified == null) {
assertNull(result);
} else {
Expand Down
Expand Up @@ -178,12 +178,12 @@ public void testGetMinimumVerifiedValue() {

// When not all results are NOT_BUILT, we should ignore NOT_BUILT.
int expResult = -1;
int result = instance.getMinimumVerifiedValue(memoryImprint, true);
int result = instance.getMinimumVerifiedValue(memoryImprint, true, Integer.MAX_VALUE);
assertEquals(expResult, result);

// Otherwise, we should use NOT_BUILT.
expResult = -4;
result = instance.getMinimumVerifiedValue(memoryImprint, false);
result = instance.getMinimumVerifiedValue(memoryImprint, false, Integer.MAX_VALUE);
assertEquals(expResult, result);
}

Expand Down Expand Up @@ -376,6 +376,35 @@ public void testGetMinimumCodeReviewValueForOneJobOverridenMixed() {
assertEquals(Integer.valueOf(2), result);
}

/**
* Tests {@link ParameterExpander#getMinimumVerifiedValue(MemoryImprint, boolean, Integer)} with two
* jobs. One successful build, the other failed missing build (null).
*
*/
@Test
public void testGetVerifiedValueOneSuccessJobAndMissingFailedJob() {
IGerritHudsonTriggerConfig config = Setup.createConfigWithCodeReviewsNull();

ParameterExpander instance = new ParameterExpander(config, jenkins);
MemoryImprint memoryImprint = mock(MemoryImprint.class);
MemoryImprint.Entry[] entries = new MemoryImprint.Entry[2];

GerritTrigger trigger = mock(GerritTrigger.class);
when(trigger.getGerritBuildFailedVerifiedValue()).thenReturn(Integer.valueOf(2));
entries[0] = Setup.createAndSetupMemoryImprintEntry(trigger, Result.SUCCESS);

trigger = mock(GerritTrigger.class);
entries[1] = Setup.createAndSetupMemoryImprintEntryWithEmptyBuild(trigger);

when(memoryImprint.getEntries()).thenReturn(entries);

// The only verified value available is of successful build,
// but score should be saturated to Failed verified value from config.
Integer result = instance.getMinimumVerifiedValue(memoryImprint, false,
config.getGerritBuildFailedVerifiedValue());
assertEquals(Integer.valueOf(config.getGerritBuildFailedVerifiedValue()), result);
}

/**
* test.
* @throws IOException IOException
Expand Down Expand Up @@ -467,6 +496,41 @@ public void testGetBuildCompletedCommandMulipleBuildsMessageOrder() throws IOExc
Setup.createPatchsetCreated(), -1, 0);
}

/**
* Test that verified value will be saturated in case of missing built.
* (Conservative approach is to assume Failed build if the build is now missing.)
*/
@Test
public void testGetBuildCompletedMissingFailedBuild() throws IOException, InterruptedException {
int buildResults = 2;
IGerritHudsonTriggerConfig config = Setup.createConfig();
Integer expectedVerifiedVote = config.getGerritBuildFailedVerifiedValue();

PatchsetCreated event = Setup.createPatchsetCreated();
TaskListener taskListener = mock(TaskListener.class);
GerritTrigger trigger = mock(GerritTrigger.class);
AbstractProject project = mock(AbstractProject.class);
Setup.setTrigger(trigger, project);
MemoryImprint.Entry[] entries = new MemoryImprint.Entry[buildResults];
EnvVars env = Setup.createEnvVars();

// Remaining successful build.
AbstractBuild build = Setup.createBuild(project, taskListener, env);
when(build.getResult()).thenReturn(Result.SUCCESS);
entries[0] = Setup.createImprintEntry(project, build);
// Missing build. Possibly Failed, but impossible to know if it is no longer available.
entries[1] = Setup.createImprintEntry(project, null);

MemoryImprint memoryImprint = mock(MemoryImprint.class);
when(memoryImprint.getEvent()).thenReturn(event);
when(memoryImprint.getEntries()).thenReturn(entries);

ParameterExpander instance = new ParameterExpander(config, jenkins);
String result = instance.getBuildCompletedCommand(memoryImprint, taskListener, null);

assertThat("Missing VERIFIED", result, containsString("VERIFIED=" + expectedVerifiedVote));
}

/**
* Sub test for {@link #testGetBuildCompletedCommandSuccessful()}.
*
Expand Down
Expand Up @@ -758,6 +758,18 @@ public static MemoryImprint.Entry createAndSetupMemoryImprintEntry(GerritTrigger
return createImprintEntry(project, build);
}

/**
* Create an MemoryImprint.Entry with a given trigger and a null build (missing).
*
* @param trigger the trigger
* @return an entry with the parameters.
*/
public static MemoryImprint.Entry createAndSetupMemoryImprintEntryWithEmptyBuild(GerritTrigger trigger) {
AbstractProject project = mock(AbstractProject.class);
setTrigger(trigger, project);
return createImprintEntry(project, null);
}

/**
* Set/mock the supplied trigger onto the supplied {@link AbstractProject} instance.
* @param trigger The trigger.
Expand Down

0 comments on commit 39403ae

Please sign in to comment.