diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java index b2b5cf338..45e36bc0e 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java @@ -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) { @@ -415,7 +418,7 @@ public Integer getMinimumVerifiedValue(MemoryImprint memoryImprint, boolean only return null; } - return verified; + return Math.min(verified, maxAllowedVerifiedValue); } /** @@ -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()) { @@ -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); } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java index 14c5728b5..9ab3bc7eb 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java @@ -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, diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java index 8016f3037..d710a4bc1 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java @@ -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 { diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java index 6e08b84da..43ea014a4 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java @@ -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); } @@ -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 @@ -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()}. * diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java index 335b66dfc..738fe9cee 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java @@ -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.