Skip to content

Adds verify method in BasePipelineTest.#463

Merged
nre-ableton merged 1 commit intojenkinsci:masterfrom
nestoracunablanco:feature/verifyMethod
Jan 10, 2022
Merged

Adds verify method in BasePipelineTest.#463
nre-ableton merged 1 commit intojenkinsci:masterfrom
nestoracunablanco:feature/verifyMethod

Conversation

@nestoracunablanco
Copy link
Copy Markdown
Contributor

@nestoracunablanco nestoracunablanco commented Dec 24, 2021

The idea behind is to provide a similar functionality to Mockito.verify: https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#1

When the verification fails, a VerificationException will be thrown, making the explicit assertions in the unit tests less necessary.

Fixes #462

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@nestoracunablanco
Copy link
Copy Markdown
Contributor Author

Hi @nre-ableton I see that you are one of the most active developers to this repo. What do you think about this change?
I am just searching for an intuitive way to avoid code duplication by adding a verify method in PipelineTestHelper.
This takes a less ambitious approach than #61 but I think there are some synergies.

Copy link
Copy Markdown
Contributor

@nre-ableton nre-ableton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good in general, but I have a few suggestions for improvement.

The idea behind is to provide a similar functionality to Mockito.verify: https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#1

When the verification fails, a VerificationException will be thrown, making the explicit assertions in the unit tests less necessary.
Copy link
Copy Markdown
Contributor

@nre-ableton nre-ableton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nre-ableton nre-ableton merged commit ee31dd9 into jenkinsci:master Jan 10, 2022
@nestoracunablanco nestoracunablanco deleted the feature/verifyMethod branch April 1, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add verify method to BasePipelineTest

2 participants