Skip to content
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

Making TAP publisher compatible with Jenkins 2 pipeline scripts + couple of fixes #12

Merged
merged 4 commits into from Aug 20, 2016

Conversation

Projects
None yet
3 participants
@anenviousguest
Copy link

commented Jul 17, 2016

Main part

... so that it's possible to use it in Jenkinsfile groovy scripts as a generic step, for example:

step([$class: "TapPublisher", testResults: "**/target/tap-unit.log"])

This ability was actually almost there. The missing part was for TapPublisher to implement SimpleBuildStep and make it and its surroundings rely on hudson.model.Run instead of hudson.model.AbstractBuild.

Minor parts

  1. Basic unit test for the publisher itself was added.
  2. Publisher constructor was not specifying default value for outputToConsole parameter.
  3. Some quirks in the URLs in "Failed tests" table. In a unit test I discovered that it was somehow containing (empty) chunk so that resulting URL was inaccessible.

Vladislav Ponomarev added some commits Jul 16, 2016

Vladislav Ponomarev
Jenkins 2-compatible publisher which can be used in pipeline scripts …
…as generic step.

Example:
```step([$class: 'TapPublisher', testResults: '*.tap'])```

Also, small fix in the "failed tests" links.
@anenviousguest

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

@kinow hi Bruno. Have you had any chance to review this?
Thanks.

@kinow

This comment has been minimized.

Copy link
Member

commented Jul 20, 2016

Hi @anenviousguest, sorry not yet. Feel free to ping me again in a few weeks if I haven't had the chance to review it. In the past I used to have sponsoring to work on some of the Jenkins plug-ins, but now it's all done in my spare time, as volunteer work, so hope you can understand that it may take a while to review it 😃

@anenviousguest

This comment has been minimized.

Copy link
Author

commented Aug 4, 2016

@kinow Hi Bruno,

Hope you're doing well.
I have just augmented PR a bit to allow merging results when more than one publisher was added to the build.
Looking forward to have you reviewed that :-)

Thanks.

@anenviousguest

This comment has been minimized.

Copy link
Author

commented Aug 16, 2016

@pjanuario

This comment has been minimized.

Copy link

commented Aug 16, 2016

Indeed, I am also migrating for the new pipeline to build nodejs app and this would really nice to have too.

@kinow

This comment has been minimized.

Copy link
Member

commented Aug 20, 2016

Looking good @anenviousguest

Tested locally with the following script successfully

stage 'Build'

node {
  sh 'cat > tap.t <<EOF\n1..2\nok 1\nok 2\nEOF'
  step([$class: "TapPublisher", testResults: "**/*.t"])
}

Will merge manually the pull request now 👍

@kinow kinow merged commit af7897b into jenkinsci:master Aug 20, 2016

1 check passed

Jenkins This pull request looks good
Details

kinow added a commit that referenced this pull request Aug 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.