Remove/deprecate checksName parameter to junit step

[jglick]

checksName was introduced in #189 as an attempt to address #183, namely

we have multiple invocations of junit (across multiple different pipelines jobs) per PR.
This means that currently, only the last reported test appears.

It suffices to use SuiteResult.getEnclosingBlockNames, which will be set automatically according to stage or parallel branch as of #76, without modifications to the Jenkinsfile.

[mrginglymus]

Posted on the PR initially, reposting here:

---

While I don't disagree that there may be a better way of detecting a default value that isn't Tests, I would be incredibly disappointed to see the ability for a user to set their own checks name at the point of junit invocation removed.

Providing it via the program that produces the xml to be consumed would require both substantial changes to our Jenkinsfiles and junit producing programs, compared to the one line we are able to achieve the behaviour with currently.

We already have an issue with the warningsng plugin where the name of the report on jenkins has to be the exact same as the check reported to github.

For context, our use case here is that we have a number of parent jobs that may run on a single PR, each with a large number of inidividual checks - e.g. our smoke run has > 10 checks reported, our long run about 5. They are a mix of test results, warnings, and coverage, plus some custom checks. They are also duplicated between parent jobs, for instance our build stage runs on both the long run and the deploy run.

Therefore, to help keep some kind of order to the large number of checks, we namespace them by the parent job, so we end up with:

Deploy  # github status, links to jenkins
Deploy / Build  # github check, links to github
Deploy / Deploy
Smoke
Smoke / Linting
Smoke / Unit Tests
Long
Long / Build
Long / Long Tests

etc.

The easiest way to achieve this, by far, is for every plugin that publishes a check to allow the user to provide the name of that check.

I'm also hoping that at some point the checks API will support updating checks, so that we can have a pattern that looks like:

withCheck('My Check Name') { check ->
    sh 'test --output output.xml'
    junit testResults: 'output.xml', check: check
}

ie, a wrapper that creates a check in progress, catches and reports exceptions.

---

Having now seen the proposal - this would be totally unacceptable as a replacement. We sometimes invoke junit multiple times per stage, so would have to totally rearchitect our jenkinsfiles to meet this reduced flexibility.

Again, I think this would make a lot of sense as a default, but the ability to override must be kept.

[mrginglymus]

Another thought arises - having the checks name have to come from the stage name will make using this check as a required check on github hard, as any refactoring of your jenkinsfile could result in a change to the computed name, requiring changes to configuration of your branch protection rules.

Again to make absolutely clear, I think this would be a sensible default (and I'm happy to raise a PR for it), but it can't be the only option.

[timja]

could you create the feature request on the checks-api plugin?

withCheck('My Check Name') { check ->
    sh 'test --output output.xml'
    junit testResults: 'output.xml', check: check
}

https://github.com/jenkinsci/checks-api-plugin/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc

it makes sense to me

[mrginglymus]

jenkinsci/checks-api-plugin#45

[jglick]

We sometimes invoke junit multiple times per stage

The design of the junit Pipeline step is that it should be run at most once per stage/branch. That is why it collects the “enclosing stage names” the way it does; and these are also preserved in URLs and the GUI—it is essentially part of the data model for the test report. You can certainly argue that it would be a better design to make this “result name” be an explicit argument to the step (which I did), but that is not how the decision went, and having two competing idioms for doing the same thing is undesirable.

At this point checksName has been released and cannot be undone, so at best it can be an override of the name implied by stage structure and used for other purposes in the plugin.

By the way I stumbled across this issue by noticing that jenkinsci/bom builds only displayed a single test report, despite running the junit step numerous times. In that context I would not particularly care about differentiating test results per branch, so long as the total count and failure details were aggregated.

[timja]

In that context I would not particularly care about differentiating test results per branch, so long as the total count and failure details were aggregated.

FTR they aren't aggregated after:
#199

[mrginglymus]

and having two competing idioms for doing the same thing is undesirable.

As far as what name is used for a check on Github, it's seems to be open season as far as the four places I'm currently it:

1. In the github branch plugin - the top level job either reports 'Jenkins' or a custom name
2. In junit - it reports either 'Tests' or a custom name (or, per your suggestion if implemented, the enclosing blocks)
3. In warnings, it reports the name of a the report (which defaults to the name of the tool, but is overridable)
4. In coverage, I'm not sure it can be set. I think it's just 'Coverage'

I would argue that as far as how jenkins interacts with external services, users should at least be given the option to chose names that suit them. Otherwise in order to have consistency on the receiving end on github, we must force the same idiom used within junit on all other plugins, and indeed on anything consuming such checks (like branch protection rules)