-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JENKINS-27395] Add a junitResults step #76
Conversation
This uses the run's externalizable ID and the step's FlowNode.getId() together as a unique key to track which SuiteResults are associated with a particular execution, and allows getting a TestResult object from a run ID and 1 or more node IDs passed to an existing TestResult object that contains suites for that run/those nodes. Note that JUnitResultArchiverTest.zip has been moved around and renamed due to jenkinsci/jenkins-test-harness@79d9603 breaking tests otherwise, since the JUnitResultArchiveTest/ directory will end up getting picked up as JENKINS_HOME otherwise.
Downstream of jenkinsci/junit-plugin#76 Also adds symbols everywhere I can, switches the thresholds to bare DataBoundConstructors (may end up doing the same for the tool types, but since they inherit from something in another plugin, it might be a bit hairier).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to see where this is going in isolation but I presume this is going to somehow allow Blue Ocean to display per-step results?
Jenkinsfile
Outdated
@@ -0,0 +1 @@ | |||
buildPlugin(jenkinsVersions: [null, '2.60.1']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for .2!
public static class DescriptorImpl extends StepDescriptor { | ||
@Override | ||
public String getFunctionName() { | ||
return "junitResults"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is going to be very hard to explain to users why they should use this instead of junit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally, I'd like to switch junit
to this, but I'm wary - the parameters are the same, so new builds should be fine, but will currently-running builds at upgrade time have problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I, uh, agree with both points. If we can retrofit the junit step to use the new APIs correctly, I'd prefer that (it's what I'd originally envisioned). But yeah we'd need to do some serious testing that it won't break things (should be viable using the workflow-cps, etc testing options though?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nod I'm not 100% sure how we can test it such that junit
means different things in a single run (albeit with a restart) but if we can find a way to do that, I'm all in favor.
} | ||
} | ||
|
||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This omits the critical functionality in #9: allowing scripts to interpret results themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
@jglick Yup, that's the intent. Via |
It's not needed - we can go from the run's TestResultAction.getResult() to any/all related test results for a list of nodes without having to add stuff to the flow graph.
This very closely matches the result laid out in the proposal I laid out on Jenkins-27395 after some conversations with Abayer, so naturally I like the general approach here. ;) -- prepare for a batch of comments when review is done though |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I really like this approach -- it's one I'd already strongly encouraged, generally, and does a much more natural job of integrating with pipeline.
I do also see some value in annotating test results with some additional descriptive metadata, as per #64 -- but that seems like it's best handled separately with a TagsAction, and with this in place we an attach that directly to the flownode where appropriate (part of the JENKINS-26522 umbrella).
This logically separates these two things in a logical way.
HOWEVER, due to the unexpected length of this and the fact that it is still WIP, I haven't had enough time to give it a thorough review to check for logic holes yet (and haven't checked the tests) -- though I've caught some things.
Approach wise though, big +1.
@@ -13,8 +13,8 @@ | |||
<description>Allows JUnit-format test results to be published.</description> | |||
<url>http://wiki.jenkins-ci.org/display/JENKINS/JUnit+Plugin</url> | |||
<properties> | |||
<jenkins.version>1.580.1</jenkins.version> | |||
<java.level>6</java.level> | |||
<jenkins.version>2.7.3</jenkins.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something here that requires 2.7.3? Might be worth holding it to 1.651 if we can since that has a pretty large install base according to stats.jenkins.io
I know, I know, it's over the year support maximum by a month or two, but I figure there's no harm in taking an slightly older core dep if we don't need the newer things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the main reason for doing so is that there'll be no real benefit for users on <2.7.3 - since the reporting will be in Blue Ocean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. In my company for example we do not use Blue Ocean because we use scripted pipeline and we have not plan conversion.
Our project has flow with two parallel stages that run integration tests on two different application server. Our needs is differentiate tests to understand from which step comes to understand on which WAS the test is failed.
Now we do know unless to archive surefire reports (xml) and check manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stats have shifted enough since July that I think we're good to bump to 2.7.3 without issue. There's still some legacy users but I'm less concerned about them getting new features added to this plugin after that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. In my company for example we do not use Blue Ocean because we use scripted pipeline and we have not plan conversion.
@nfalco79 Sorry, but I don't see the relationship, FWIW. You can (and we definitely do) use Blue Ocean and it will render any Pipeline, be it Scripted or Declarative.
Sorry to digress, but I thought I wouldn't leave that statement without an answer for possibly passing-by people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@batmat that i remember blue ocean can render scripted pipeline only if non "standard" steps are written into a sort of step wrapper. And if i remember rigth also statements like if/def variable or function/for cycles and other groovy commands cause error in the parser when I had open the page of the our pipeline. So I had remove blue ocean and 15 related plugin after 3 hours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not denying you could have hit some bug. If you're able to report it, that would be great. But yes, we use Blue Ocean in many many places, definitely too using Scripted pipelines, so this does work normally. Not being the case looks like this would be great to be reported somewhere.
@@ -241,7 +241,7 @@ public void setData(List<Data> testData) { | |||
/** | |||
* Merges an additional test result into this one. | |||
*/ | |||
void mergeResult(TestResult additionalResult, TaskListener listener) { | |||
public void mergeResult(TestResult additionalResult, TaskListener listener) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need to be public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, used over in xunit
.
TestResultAction action = parseAndAttach(this, null, build, workspace, launcher, listener); | ||
|
||
if (action != null && action.getResult().getFailCount() > 0) | ||
build.setResult(Result.UNSTABLE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't feel right to me -- don't people tend to handle the way test results map to build results in vastly different ways, generally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I waffled on this a lot. This is something that I'd like to differently, but until https://issues.jenkins-ci.org/browse/JENKINS-43995 lands, we don't have a way to pick up that UNSTABLE
status to cover the whole build later (modulo try/catch
and the like). So for the moment, I'd say go with principle of least surprise: if you use junit
now, it sets the build to UNSTABLE
, so junitResults
should do the same.
|
||
return r; | ||
} | ||
|
||
private static void parseSuite(File xmlReport, boolean keepLongStdio, List<SuiteResult> r, Element root) throws DocumentException, IOException { | ||
private static void parseSuite(File xmlReport, boolean keepLongStdio, List<SuiteResult> r, Element root, String runId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend making heavy use of CheckForNull in the runId and nodeId fields/parameters we've added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
* @since 1.21 | ||
*/ | ||
@Exported(visibility=9) | ||
public String getNodeId() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CheckForNull (here and abvoe)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nod
@@ -67,6 +71,11 @@ | |||
private transient Map<String,SuiteResult> suitesByName; | |||
|
|||
/** | |||
* {@link #suites} keyed by their run ID and node ID for faster lookup. May be empty. | |||
*/ | |||
private transient Map<String,Map<String,List<SuiteResult>>> suitesByRunAndNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeaaaaah, I don't love it, but hey, it works.
List<SuiteResult> suites = suitesByRunAndNode.get(runId).get(n); | ||
if (suites != null) { | ||
for (SuiteResult s : suites) { | ||
s.setParent(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely clear what the implications of setParent are here -- it feels rather odd to see this in the getter though, since isn't it changing the persistent state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I cargo-culted this from merge
, since the TestResult
we return is similar, but it's probably without a point, yeah.
@@ -712,6 +798,16 @@ public void freeze(TestResultAction parent) { | |||
pr.freeze(); | |||
} | |||
|
|||
private void addSuiteByRunAndNode(SuiteResult s) { | |||
if (suitesByRunAndNode.get(s.getRunId()) == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block is rather confusing to follow, because you're doing successive gets and puts, please could you consider restructuring with nesting of conditionals so it's clear where the flow of execution will go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, it's evolved a bit awkwardly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. At least at first glance, I'm honestly not sure how to restructure this. For the moment, I'll add comments and if there's something you or anyone else sees as a better approach to the same end result, I'll happily embrace it.
public static class DescriptorImpl extends StepDescriptor { | ||
@Override | ||
public String getFunctionName() { | ||
return "junitResults"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I, uh, agree with both points. If we can retrofit the junit step to use the new APIs correctly, I'd prefer that (it's what I'd originally envisioned). But yeah we'd need to do some serious testing that it won't break things (should be viable using the workflow-cps, etc testing options though?)
Hmm...anyone know if it's possible to just automatically use the help and |
public static class DescriptorImpl extends StepDescriptor { | ||
@Override | ||
public String getFunctionName() { | ||
return "junit"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to pick something different, as this will clash with the symbol aliased to step([$class: 'JUnitResultsArchiver', …])
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check elsewhere in the PR - I switched that symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check elsewhere in the PR - I switched that symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check elsewhere in the PR - I switched that symbol.
<dependency> | ||
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-job</artifactId> | ||
<version>2.11.1</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipeline deps should be optional
... whilst I have no reason not to install pipeline, just because I use junit I should not require it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but decided we're far enough along in the Pipeline era now that it's eminently reasonable to assume that Pipeline be installed everywhere. =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @jtnord , you should not mark as not optional dependency only because you assume pipeline is installed everywhere because with this trend jenkins became a big monolite where installing one plugin downloads all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abayer NO NO. ping me offline if you want to be educated why pipeline will never be installed on ALL jenkins instances, and why doing this in junit is pure evil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipeline must be optional.
Remember this is a plugin split from core so often you have no choice but to install this if you depend on a plugin built on an old core. (one example is the AD plugin)
Aaaaahhh, good point re split from core. Will change to optional. |
Conflicts: pom.xml src/test/java/hudson/tasks/junit/JUnitResultArchiverTest.java src/test/resources/hudson/tasks/junit/JUnitResultArchiverTest.zip src/test/resources/hudson/tasks/junit/JUnitResultArchiverTest/All.zip src/test/resources/hudson/tasks/junit/JUnitResultArchiverTest/testData.zip
List<String> enclosingBlocks = new ArrayList<>(); | ||
for (FlowNode enclosing : node.getEnclosingBlocks()) { | ||
if (enclosing != null && enclosing.getAction(LabelAction.class) != null) { | ||
if ((enclosing instanceof StepStartNode && ((StepStartNode) enclosing).getDescriptor() instanceof StageStep.DescriptorImpl) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this not blow up if pipeline-stage-step
is not installed (it is optional)...
Can you install workflow-job
without pipeline-stage-step
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Either we make the pipeline dependencies optional and accept that things blow up (FlowNode
from workflow-api
too!) or we make them non optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about move the specific pipeline step executor to a separate plugin like
- docker-build-step-plugin
- workflow-step-api-plugin
- pipeline-maven-plugin
- XYZ-step-plugin
and so on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the same reason those plugin are created. Move pipeline dependency far from base plugin. For the same reason do not force dependency from Jenkins 2.7.3 because
So the main reason for doing so is that there'll be no real benefit for users on <2.7.3 - since the reporting will be in Blue Ocean.
than move this step to one of Blue Ocean plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was 2.5 years ago - a lot has changed since then, most notably the end of bundling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abayer so this should not blow up. If you can not use this step without the pipeline-stage-step
then the step should not be active (ie you can not use it), or you can use it but you get no "per stage" results.
You should be able to do this, there are many ways to do it.
one would be to check if the plugin is installed and only then call some code in a different class.
I'm not sure if Steps can be marked as Optional using the varient plugin or the like to say this is only active if this plugin is installed, but I guess it should be possible, if not I think we should be able to do this. another way is called out here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going with the variant
approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the variant approach but if it deals with this I'm all for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually @abayer you could just have it look for the descriptor ID matching, which means you don't need a hard dependency on stage-step (string equals vs instanceof).
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering why you need script-security - you don;t call any groovy...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hudson.tasks.junit.TestResultSummary
uses the @Whitelisted
annotation.
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-cps</artifactId> | ||
<version>${workflow-cps.version}</version> | ||
<optional>true</optional> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here - you do not seem to need to depend on this, you just want an optional on workflow job do you not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hudson.tasks.junit.pipeline.JUnitResultsStepExecution
uses org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode
return new JUnitResultsStepExecution(this, context); | ||
} | ||
|
||
@Extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so you currently require "pipeline-stage-step" as optional - so this should be marked as optional depending on the installation of that plugin - or you need to make the code not blow up.
Check the Varient plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nifty!
<dependency> | ||
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-step-api</artifactId> | ||
<version>2.12</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was pipeline going to be an optional dependency or not?
I'm confused about what all has happened here since I built the APIs to let you fetch enclosing blocks for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this one just missed an optional?
return node.getDisplayName(); | ||
} | ||
} catch (Exception e) { | ||
// TODO: Something in that odd case where we can get an NPE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, NPE where? It looks like this is null protected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
execution.getNode
can theoretically throw an NPE. =)
@abayer I'm unclear if this one is ready for review again, but it looks pretty reasonable to me modulo some funkiness around dependency handling. |
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>script-security</artifactId> | ||
<version>1.30</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this required for junit "classic"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not historically - hudson.tasks.junit.TestResultSummary
, which is added here for having something we can return from a junit
step call, needs @Whitelisted
. The class isn't an Extension
, so my gut reaction is that we shouldn't do an optional dependency, but if I'm wrong I'm happy to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - one question about
<artifactId>script-security</artifactId>
being non-optional.
Downstream of jenkinsci/junit-plugin#76. This records the `FlowNode#id` for the step that's actually recording the test results and the enclosing stage(s)/branch(es) of that `FlowNode` with each suite, along with the names of the enclosing stage(s)/branch(es) on each `CaseResult` for display purposes. This is accomplished by getting the `FlowNode` via `context.get(FlowNode.class)`, and then passing the configured `JUnitResultArchiver`, the node's ID, the enclosing stage/branch IDs, and the run (which is used to get the `Run#getExternalizableId()` for internal purposes) to the new `JUnitResultArchiver#parseAndAttach` static method. That method does the rest of the work. Once this is all released, the classic test results pages will display the test name with the enclosing stage/branch names prepended (if there are test results from multiple stages/branches in the full test result - i.e., only if relevant). Blue Ocean has a PR to do the same that's pending.
* [JENKINS-27395] Record flow node and enclosing stage/branch for tests Downstream of jenkinsci/junit-plugin#76. This records the `FlowNode#id` for the step that's actually recording the test results and the enclosing stage(s)/branch(es) of that `FlowNode` with each suite, along with the names of the enclosing stage(s)/branch(es) on each `CaseResult` for display purposes. This is accomplished by getting the `FlowNode` via `context.get(FlowNode.class)`, and then passing the configured `JUnitResultArchiver`, the node's ID, the enclosing stage/branch IDs, and the run (which is used to get the `Run#getExternalizableId()` for internal purposes) to the new `JUnitResultArchiver#parseAndAttach` static method. That method does the rest of the work. Once this is all released, the classic test results pages will display the test name with the enclosing stage/branch names prepended (if there are test results from multiple stages/branches in the full test result - i.e., only if relevant). Blue Ocean has a PR to do the same that's pending. * Add issue annotation to the new test * Bump to junit 1.23 release. * Bump parent plugin version, fix tons of dependency issues * [JENKINS-49482] Hide empty 'Deployed Artifacts:' (jenkinsci#124) * [JENKINS-49482] Store artifact deployment details in the database and fix display of deployed artifacts. (jenkinsci#125) * [maven-release-plugin] prepare release pipeline-maven-3.3.1-beta-1 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49482] Fix "java.lang.NoClassDefFoundError: edu/emory/mathcs/backport/java/util/Collections" * [maven-release-plugin] prepare release pipeline-maven-3.3.1-beta-2 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49482] fix configuration screen * [maven-release-plugin] prepare release pipeline-maven-3.3.1 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49549] Only display once the "Deployed Artifacts" report if withMaven is invoked 2+ times in a pipeline * [maven-release-plugin] prepare release pipeline-maven-3.3.2 * [maven-release-plugin] prepare for next development iteration * [JENKINS-27395] temporarily disable bump to the jenkins-junit-pipeline 1.23 due to additional dependencies. * Reverting d95e3dc as it is not necessary after ba36c1b. * [JENKINS-46785] Strategy to enable / disable publishers in pipelines. Default strategy, "IMPLICIT" is the current strategy to implicitly enable all publishers unless they are explicitly disabled. The second available strategy, "EXPLICIT", requires to explicitly enable the publishers in the "withMaven(options: [...])" attribute * [JENKINS-46785] Add unit test * [maven-release-plugin] prepare release pipeline-maven-3.4.0-beta-1 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49632] Also record generated attached artifacts * [JENKINS-49632] uncomment the code * [maven-release-plugin] prepare release pipeline-maven-3.4.0 * [maven-release-plugin] prepare for next development iteration * [maven-release-plugin] prepare release pipeline-maven-3.4.1 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49898] maven-invoker-plugin add support for relative path in "reportsDirectory", "projectsDirectory" and "cloneProjectsTo" (jenkinsci#131) * [maven-release-plugin] prepare release pipeline-maven-3.4.2 * [maven-release-plugin] prepare for next development iteration * [JENKINS-49896] Add support for invoker:integration-test in addition to invoker:run * [maven-release-plugin] prepare release pipeline-maven-3.4.3 * [maven-release-plugin] prepare for next development iteration * change order of developers in pom.xml so that Cyrille Le Clerc gets assigned the Jira issues * bump maven pom.xml plugins and parent pom * [JENKINS-50241] Introduce a dedicated build result screen for Maven details * JENKINS-49721 support `mvnw` maven installation (jenkinsci#136) * SQL views to help troubleshooting * [JENKINS-50241] Add "dependencies" section on the Maven details screen (jenkinsci#137)
<version>2.22</version> | ||
<optional>true</optional> | ||
<exclusions> | ||
<exclusion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#111 FTR
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-api</artifactId> | ||
<version>2.22</version> | ||
<optional>true</optional> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#88 FTR
JENKINS-27395
I like this one better than #75 (which I'm closing) - it doesn't subsume #64, and doesn't bother with that test run name stuff at all, instead relying on Pipeline itself to get the right information to mark
SuiteResult
s as associated with a particularRun
andFlowNode
when appropriate.This uses the run's externalizable ID and the step's FlowNode.getId()
together as a unique key to track which SuiteResults are associated
with a particular execution, and allows getting a TestResult object
from a run ID and 1 or more node IDs passed to an existing TestResult
object that contains suites for that run/those nodes.
Note that JUnitResultArchiverTest.zip has been moved around and
renamed due to
jenkinsci/jenkins-test-harness@79d9603
breaking tests otherwise, since the JUnitResultArchiveTest/ directory
will end up getting picked up as JENKINS_HOME otherwise.
junit
stepTestResultAction
CaseResult#getDisplayName()
getFailedSince()
(DONE)safeName
)cc @reviewbybees