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

Ensure we always call getContext().onFailure() in stop #73

Merged
merged 1 commit into from
Jan 27, 2021

Conversation

mrginglymus
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 27, 2021

Codecov Report

Merging #73 (8215be6) into master (629cd7e) will decrease coverage by 0.18%.
The diff coverage is 16.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #73      +/-   ##
============================================
- Coverage     87.23%   87.05%   -0.19%     
  Complexity       86       86              
============================================
  Files            15       15              
  Lines           478      479       +1     
  Branches         24       25       +1     
============================================
  Hits            417      417              
- Misses           54       55       +1     
  Partials          7        7              
Impacted Files Coverage Δ Complexity Δ
...o/jenkins/plugins/checks/steps/WithChecksStep.java 75.58% <16.66%> (-0.89%) 2.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 629cd7e...8215be6. Read the comment docs.

@timja timja added the bug Something isn't working label Jan 27, 2021
@timja
Copy link
Member

timja commented Jan 27, 2021

@mrginglymus have you tested this fixes your issue? or you want to deploy the incremental for a bit?

@mrginglymus
Copy link
Contributor Author

I'll chuck the incremental on and see how it goes. Seems to repro fairly reliably. It'd be nice to test it, but I'm not sure how best to do that.

@mrginglymus
Copy link
Contributor Author

It seems like I need to run a job and abort it externally - do we have a pattern for that?

@timja
Copy link
Member

timja commented Jan 27, 2021

I'll chuck the incremental on and see how it goes. Seems to repro fairly reliably. It'd be nice to test it, but I'm not sure how best to do that.

not needed imo

@mrginglymus
Copy link
Contributor Author

Initial impressions suggest that yes, this has fixed the issue.

@timja timja merged commit d73ab46 into jenkinsci:master Jan 27, 2021
@mrginglymus mrginglymus deleted the missing-on-failure branch January 27, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants