-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add capturing checks publisher, make publishChecks withChecksName aware #55
Add capturing checks publisher, make publishChecks withChecksName aware #55
Conversation
e2fbada
to
1c2a731
Compare
/** | ||
* Tests that the step "publishChecks" can be used in pipeline script. | ||
* | ||
* @throws IOException if fails get log from run | ||
*/ | ||
@SuppressWarnings("OptionalGetWithoutIsPresent") |
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 would just extract the output to a variable and chuck and orElseThrow instead of .get
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'll be a lot of orElseThrows
! Might make it a bit less readable.
Is there a way to shift this code into the |
Do we have any other places that need it? |
There's a few plugins that ship jars for this under the classifier 'tests' jcasc used to, and some of the pipeline ones do, search the bom for the tests classifier: https://github.com/jenkinsci/bom |
Yes, just move the code to the |
It's only needed in tests, but it will be useful for testing in the consumers (I'm adding some details to |
docs/consumers-guide.md
Outdated
in the `test` classifier, as `io.jenkins.plugins.checks.api.test.CapturingChecksPublisher`. | ||
|
||
Adding the factory for this publisher as a `TestExtension` will allow inspection of published checks after running a job | ||
on a `JenkinsRule`. |
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 would add the link from the javadoc here as well,
to io.jenkins.plugins.checks.steps.PublishChecksStepITest
|
||
ChecksDetails details = PUBLISHER_FACTORY.getPublishedChecks().get(0); | ||
|
||
assertThat(details.getName().get()).isEqualTo("customized-check"); |
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.
Typically, one would use in AssertJ:
assertThat(details.getName().get()).isEqualTo("customized-check"); | |
assertThat(details.getName()).isPresent().get().isEqualTo("customized-check"); |
This will also not require the annotation.
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.
Oh, neat! Thanks.
ChecksDetails extractChecksDetails() throws IOException, InterruptedException { | ||
// If a checks name has been provided as part of the step, use that. | ||
// If not, check to see if there is an active ChecksInfo context (e.g. from withChecks). | ||
String checksName = Optional.ofNullable(Util.fixEmpty(step.getName())).orElse( |
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 better grammar
String checksName = Optional.ofNullable(Util.fixEmpty(step.getName())).orElse( | |
StringUtils.defaultIfEmpty(step.getName(), | |
Optional.ofNullable(getContext().get(ChecksInfo.class)) | |
.map(ChecksInfo::getName) | |
.orElse(StringUtils.EMPTY)) |
0614113
to
60c6dd3
Compare
New mystery - why is the Windows build hanging between reporting integration test results and running the run-revapi step... |
Not a big deal, there are always some problems with the windows build on our community ci (due to resources and so on); normally I only take a look at the github builds and the linux build on the community server. |
<executions> | ||
<execution> | ||
<goals> | ||
<goal>test-jar</goal> |
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.
@XiongKezhi this is normally accomplished just with
<no-test-jar>false</no-test-jar>
For some reason you are inheriting from analysis-pom
, which does things differently, rather than directly from org.jenkins-ci.plugins:plugin
.
Addresses #54