Skip to content

Commit 1a10f56

Browse files
uhafneruhafner
authored
Merge pull request #1970 from jenkinsci/JENKINS-75344-fix-custom-url
[JENKINS-75344] Fix custom URL name of result action
2 parents 1b20ab1 + dd29755 commit 1a10f56

File tree

11 files changed

+159
-69
lines changed

11 files changed

+159
-69
lines changed

‎.github/workflows/ui-tests.yml

+5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ jobs:
1919

2020
steps:
2121
- uses: actions/checkout@v4
22+
if: github.event_name == 'push'
23+
- uses: actions/checkout@v4
24+
with:
25+
ref: "${{ github.event.pull_request.merge_commit_sha }}"
26+
if: github.event_name == 'pull_request_target'
2227
- name: Set up JDK 21
2328
uses: actions/setup-java@v4
2429
with:

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ResultAction.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
package io.jenkins.plugins.analysis.core.model;
22

3-
import java.io.Serializable;
4-
import java.nio.charset.Charset;
5-
import java.util.Collection;
6-
import java.util.Collections;
7-
83
import org.apache.commons.lang3.StringUtils;
94

105
import edu.hm.hafner.analysis.Issue;
@@ -14,6 +9,11 @@
149
import edu.umd.cs.findbugs.annotations.CheckForNull;
1510
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1611

12+
import java.io.Serializable;
13+
import java.nio.charset.Charset;
14+
import java.util.Collection;
15+
import java.util.Collections;
16+
1717
import org.kohsuke.stapler.StaplerProxy;
1818
import org.kohsuke.stapler.bind.JavaScriptMethod;
1919
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
@@ -164,7 +164,7 @@ public String getDisplayName() {
164164

165165
@Override
166166
public String getUrlName() {
167-
return getLabelProvider().getId();
167+
return StringUtils.defaultIfEmpty(id, getLabelProvider().getId());
168168
}
169169

170170
/**

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java

+13-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
package io.jenkins.plugins.analysis.core.steps;
22

3+
import org.apache.commons.lang3.StringUtils;
4+
5+
import edu.hm.hafner.analysis.Severity;
6+
import edu.hm.hafner.util.FilteredLog;
7+
import edu.umd.cs.findbugs.annotations.CheckForNull;
8+
import edu.umd.cs.findbugs.annotations.NonNull;
9+
310
import java.io.IOException;
411
import java.nio.charset.Charset;
512
import java.util.ArrayList;
@@ -9,13 +16,6 @@
916
import java.util.Set;
1017
import java.util.stream.Collectors;
1118

12-
import org.apache.commons.lang3.StringUtils;
13-
14-
import edu.hm.hafner.analysis.Severity;
15-
import edu.hm.hafner.util.FilteredLog;
16-
import edu.umd.cs.findbugs.annotations.CheckForNull;
17-
import edu.umd.cs.findbugs.annotations.NonNull;
18-
1919
import org.kohsuke.stapler.AncestorInPath;
2020
import org.kohsuke.stapler.DataBoundConstructor;
2121
import org.kohsuke.stapler.DataBoundSetter;
@@ -680,12 +680,12 @@ private List<AnalysisResult> record(final Run<?, ?> run, final FilePath workspac
680680
Tool tool = analysisTools.get(0);
681681

682682
var customId = StringUtils.defaultIfBlank(getId(), tool.getActualId());
683-
AnnotatedReport report = new AnnotatedReport(customId);
684-
report.add(scanWithTool(run, workspace, listener, tool), tool.getActualId());
685-
686683
var customName = StringUtils.defaultIfBlank(getName(), tool.getActualName());
687684
var customIcon = StringUtils.defaultIfBlank(getIcon(), tool.getIcon());
688685

686+
AnnotatedReport report = new AnnotatedReport(customId);
687+
report.add(scanWithTool(run, workspace, listener, tool), tool.getActualId());
688+
689689
results.add(publishResult(run, workspace, listener, customName,
690690
report, customName, customIcon, resultHandler));
691691

@@ -714,9 +714,9 @@ private List<AnalysisResult> record(final Run<?, ?> run, final FilePath workspac
714714
results.add(publishResult(run, workspace, listener, tool.getActualName(),
715715
report, getReportName(tool), tool.getIcon(), resultHandler));
716716
}
717-
}
718-
if (StringUtils.isNotBlank(getId()) || !StringUtils.isNotBlank(getName()) || !StringUtils.isNotBlank(getIcon())) {
719-
logHandler.log("Do not set id, name, or icon for both the tool and the recorder");
717+
if (StringUtils.isNotBlank(getId()) || StringUtils.isNotBlank(getName()) || StringUtils.isNotBlank(getIcon())) {
718+
logHandler.log("Do not set id, name, or icon of recorder when multiple tools are defined");
719+
}
720720
}
721721
}
722722
return results;

‎plugin/src/main/webapp/js/issues-sunburst.js

+5
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@
7575
color: '#ddd',
7676
borderWidth: 2
7777
},
78+
emphasis: {
79+
itemStyle: {
80+
color: 'inherit'
81+
}
82+
},
7883
label: {
7984
rotate: 'horizontal'
8085
}

‎plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/StepsITest.java

+46-19
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,5 @@
11
package io.jenkins.plugins.analysis.warnings.steps;
22

3-
import java.io.File;
4-
import java.io.IOException;
5-
import java.nio.file.Files;
6-
import java.nio.file.Path;
7-
import java.util.ArrayList;
8-
import java.util.Collections;
9-
import java.util.List;
10-
import java.util.Objects;
11-
import java.util.Set;
12-
import java.util.stream.Collectors;
13-
143
import org.eclipse.collections.impl.factory.Lists;
154
import org.junit.jupiter.api.DisplayName;
165
import org.junit.jupiter.api.Test;
@@ -22,6 +11,17 @@
2211
import edu.hm.hafner.analysis.Report;
2312
import edu.hm.hafner.analysis.Severity;
2413

14+
import java.io.File;
15+
import java.io.IOException;
16+
import java.nio.file.Files;
17+
import java.nio.file.Path;
18+
import java.util.ArrayList;
19+
import java.util.Collections;
20+
import java.util.List;
21+
import java.util.Objects;
22+
import java.util.Set;
23+
import java.util.stream.Collectors;
24+
2525
import org.kohsuke.stapler.HttpResponse;
2626
import org.jenkinsci.Symbol;
2727
import org.jenkinsci.plugins.workflow.actions.WarningAction;
@@ -580,37 +580,64 @@ void shouldFailBuildWhenFailBuildOnErrorsIsSet() {
580580
scheduleBuildAndAssertStatus(job, Result.FAILURE);
581581
}
582582

583-
@Test
584-
void shouldReportResultWithDifferentIdNameAndIconInStep() {
585-
WorkflowJob job = createPipelineWithWorkspaceFilesWithSuffix("emptyFile.txt");
583+
@ParameterizedTest(name = "{index} => Reading JavaDoc warnings from file \"{0}\"")
584+
@ValueSource(strings = {"javadoc.txt", "emptyFile.txt"})
585+
@org.junitpioneer.jupiter.Issue("JENKINS-75344")
586+
void shouldReportResultWithDifferentIdNameAndIconInStep(final String fileName) {
587+
WorkflowJob job = createPipelineWithWorkspaceFilesWithSuffix(fileName);
586588

587589
job.setDefinition(asStage(
588590
"recordIssues id: 'custom-id', name: 'custom-name', icon: 'custom-icon', "
589591
+ "tool: javaDoc(pattern:'**/*issues.txt', reportEncoding:'UTF-8')"));
590592

591593
var action = getResultAction(buildWithResult(job, Result.SUCCESS));
594+
assertThat(action.getUrlName()).isEqualTo("custom-id");
592595
assertThat(action.getId()).isEqualTo("custom-id");
593596
assertThat(action.getDisplayName()).startsWith("custom-name");
594597
assertThat(action.getIconFileName()).isEqualTo("custom-icon");
595598
}
596599

597-
@Test
598-
void shouldReportResultWithDifferentIdNameAndIconInTool() {
599-
WorkflowJob job = createPipelineWithWorkspaceFilesWithSuffix("emptyFile.txt");
600+
@ParameterizedTest(name = "{index} => Reading JavaDoc warnings from file \"{0}\"")
601+
@ValueSource(strings = {"javadoc.txt", "emptyFile.txt"})
602+
@org.junitpioneer.jupiter.Issue("JENKINS-75344")
603+
void shouldReportResultWithDifferentIdNameAndIconInTool(final String fileName) {
604+
WorkflowJob job = createPipelineWithWorkspaceFilesWithSuffix(fileName);
600605

601606
job.setDefinition(asStage(
602607
"recordIssues tool: javaDoc(pattern:'**/*issues.txt', reportEncoding:'UTF-8',"
603608
+ "id: 'custom-id', name: 'custom-name', icon: 'custom-icon')"));
604609

605610
var action = getResultAction(buildWithResult(job, Result.SUCCESS));
611+
assertThat(action.getUrlName()).isEqualTo("custom-id");
612+
assertThat(action.getId()).isEqualTo("custom-id");
613+
assertThat(action.getDisplayName()).startsWith("custom-name");
614+
assertThat(action.getIconFileName()).isEqualTo("custom-icon");
615+
}
616+
617+
@ParameterizedTest(name = "{index} => Reading JavaDoc warnings from file \"{0}\"")
618+
@ValueSource(strings = {"javadoc.txt", "emptyFile.txt"})
619+
@org.junitpioneer.jupiter.Issue("JENKINS-75391")
620+
void shouldShowWarningWhenUsingIdForToolAndRecorder(final String fileName) {
621+
WorkflowJob job = createPipelineWithWorkspaceFilesWithSuffix(fileName);
622+
623+
job.setDefinition(asStage(
624+
"recordIssues id: 'custom-id', name: 'custom-name', icon: 'custom-icon', "
625+
+ "tool: javaDoc(pattern:'**/*issues.txt', reportEncoding:'UTF-8',"
626+
+ "id: 'custom-id', name: 'custom-name', icon: 'custom-icon')"));
627+
628+
var action = getResultAction(buildWithResult(job, Result.SUCCESS));
629+
assertThat(action.getUrlName()).isEqualTo("custom-id");
606630
assertThat(action.getId()).isEqualTo("custom-id");
607631
assertThat(action.getDisplayName()).startsWith("custom-name");
608632
assertThat(action.getIconFileName()).isEqualTo("custom-icon");
633+
634+
assertThat(getConsoleLog(action.getOwner()))
635+
.contains("Do not set id, name, or icon for both the tool and the recorder");
609636
}
610637

611638
/**
612639
* Runs the all Java parsers on three output files: the build should report issues of all tools. The results should
613-
* be aggregated into a new action with the specified ID. Since no name is given the default name is used.
640+
* be aggregated into a new action with the specified ID. Since no name is given, the default name is used.
614641
*/
615642
@Test
616643
void shouldProvideADefaultNameIfNoOneIsGiven() {
@@ -926,7 +953,7 @@ void shouldLogWarningIfNameIsSetWhenNotAggregating() {
926953
assertThat(ids).containsExactly("groovy-1", "groovy-2");
927954

928955
assertThat(getConsoleLog(build))
929-
.contains("Do not set id, name, or icon for both the tool and the recorder");
956+
.contains("Do not set id, name, or icon of recorder when multiple tools are defined");
930957
}
931958

932959
/**

‎ui-tests/src/main/java/io/jenkins/plugins/analysis/warnings/AnalysisResult.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
package io.jenkins.plugins.analysis.warnings;
22

3-
import java.net.URL;
4-
import java.util.Collection;
5-
import java.util.List;
6-
import java.util.NoSuchElementException;
7-
import java.util.stream.Collectors;
8-
93
import org.apache.commons.lang3.StringUtils;
104
import org.openqa.selenium.By;
115
import org.openqa.selenium.WebElement;
@@ -15,6 +9,12 @@
159

1610
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1711

12+
import java.net.URL;
13+
import java.util.Collection;
14+
import java.util.List;
15+
import java.util.NoSuchElementException;
16+
import java.util.stream.Collectors;
17+
1818
import org.jenkinsci.test.acceptance.po.Build;
1919
import org.jenkinsci.test.acceptance.po.PageObject;
2020

@@ -217,7 +217,7 @@ public ForensicsTable openForensicsTable() {
217217
* @return the instance of the PageObject to which the link leads to
218218
*/
219219
public <T extends PageObject> T openLinkOnSite(final WebElement element, final Class<T> type) {
220-
String link = element.getAttribute("href");
220+
String link = element.getDomAttribute("href");
221221
T retVal = newInstance(type, injector, url(link));
222222
element.click();
223223
return retVal;
@@ -229,7 +229,7 @@ public <T extends PageObject> T openLinkOnSite(final WebElement element, final C
229229
* @return the select-element where the user can choose how many rows should be displayed
230230
*/
231231
public Select getLengthSelectElementByActiveTab() {
232-
var element = find(By.xpath(ACTIVE_TAB + "select[contains(@name, 'length')]"));
232+
var element = find(By.xpath(ACTIVE_TAB + "select[@id[starts-with(., 'dt-length')]]"));
233233
return new Select(element);
234234
}
235235

‎ui-tests/src/main/java/io/jenkins/plugins/analysis/warnings/AnalysisSummary.java

+15-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
package io.jenkins.plugins.analysis.warnings;
22

3+
import org.apache.commons.lang3.StringUtils;
4+
import org.openqa.selenium.By;
5+
import org.openqa.selenium.WebElement;
6+
37
import java.util.Arrays;
48
import java.util.Collections;
59
import java.util.List;
@@ -9,10 +13,6 @@
913
import java.util.regex.Pattern;
1014
import java.util.stream.Collectors;
1115

12-
import org.apache.commons.lang3.StringUtils;
13-
import org.openqa.selenium.By;
14-
import org.openqa.selenium.WebElement;
15-
1616
import org.jenkinsci.test.acceptance.po.Build;
1717
import org.jenkinsci.test.acceptance.po.PageObject;
1818

@@ -34,6 +34,7 @@ public class AnalysisSummary extends PageObject {
3434
private final WebElement infoElement;
3535

3636
private final List<WebElement> results;
37+
private final WebElement summary;
3738

3839
/**
3940
* Creates a new page object representing the analysis summary on the build page of a job.
@@ -49,7 +50,7 @@ public AnalysisSummary(final Build parent, final String id) {
4950

5051
this.id = id;
5152

52-
WebElement summary = getElement(By.id(id + "-summary"));
53+
summary = getElement(By.id(id + "-summary"));
5354
titleElement = summary.findElement(By.id(id + "-title"));
5455

5556
infoElement = summary.findElement(By.className("fa-image-button"));
@@ -70,6 +71,15 @@ public String getTitleText() {
7071
return titleElement.getText();
7172
}
7273

74+
/**
75+
* Return the image the summary. Note that not all tools use an image. Symbols are not supported by this method.
76+
*
77+
* @return the image
78+
*/
79+
public String getImage() {
80+
return summary.findElement(by.xpath("../../td/img")).getDomAttribute("src");
81+
}
82+
7383
/**
7484
* Returns the number of new issues.
7585
*

‎ui-tests/src/test/java/io/jenkins/plugins/analysis/warnings/DetailsTabUiTest.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
package io.jenkins.plugins.analysis.warnings;
22

3-
import java.time.Duration;
4-
import java.util.Collection;
5-
import java.util.List;
6-
73
import org.junit.Test;
84
import org.openqa.selenium.WebElement;
95
import org.openqa.selenium.support.ui.ExpectedConditions;
@@ -12,6 +8,10 @@
128

139
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1410

11+
import java.time.Duration;
12+
import java.util.Collection;
13+
import java.util.List;
14+
1515
import org.jenkinsci.test.acceptance.junit.WithPlugins;
1616
import org.jenkinsci.test.acceptance.po.Build;
1717
import org.jenkinsci.test.acceptance.po.FreeStyleJob;
@@ -33,7 +33,7 @@ public class DetailsTabUiTest extends UiTest {
3333
private static final String DETAILS_TAB_RESOURCES = "details_tab_test/";
3434

3535
/**
36-
* When a single warning is being recognized only the issues-tab should be shown.
36+
* When a single warning is being recognized, only the issues-tab should be shown.
3737
*/
3838
@Test
3939
public void shouldPopulateDetailsTabSingleWarning() {
@@ -56,7 +56,7 @@ public void shouldPopulateDetailsTabSingleWarning() {
5656
}
5757

5858
/**
59-
* When two warnings are being recognized in one file the tabs issues, files and folders should be shown.
59+
* When two warnings are being recognized in one file, then the tabs "issues", "files", and "folders" should be shown.
6060
*/
6161
@Test
6262
public void shouldPopulateDetailsTabMultipleWarnings() {

‎ui-tests/src/test/java/io/jenkins/plugins/analysis/warnings/SmokeTests.java

-10
Original file line numberDiff line numberDiff line change
@@ -178,16 +178,6 @@ private void verifyDetailsTab(final Build build) {
178178
assertThat(resultPage.getPaginationButtons()).hasSize(1);
179179
}
180180

181-
private StringBuilder createReportFilesStep(final WorkflowJob job, final int build) {
182-
String[] fileNames = {"checkstyle-report.xml", "pmd-report.xml", "findbugsXml.xml", "cpd.xml", "Main.java", "pep8Test.txt"};
183-
StringBuilder resourceCopySteps = new StringBuilder();
184-
for (String fileName : fileNames) {
185-
resourceCopySteps.append(job.copyResourceStep(
186-
"/build_status_test/build_0" + build + "/" + fileName).replace("\\", "\\\\"));
187-
}
188-
return resourceCopySteps;
189-
}
190-
191181
@Override
192182
protected IssuesRecorder addAllRecorders(final FreeStyleJob job) {
193183
IssuesRecorder issuesRecorder = super.addAllRecorders(job);

0 commit comments

Comments
 (0)