Skip to content

Conversation

@strangelookingnerd
Copy link
Contributor

This PR aims to migrate all tests to JUnit5. Changes include:

  • Migrate annotations and imports
  • Migrate assertions
  • Remove public visibility for test classes and methods
  • Minor clean up

I am well aware that this is a quite large changeset however I hope that there is still interest in this PR and it will be reviewed.
If there are any questions, please do not hesitate to ping me.

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

@strangelookingnerd strangelookingnerd force-pushed the migrate_to_junit5 branch 2 times, most recently from 488aad2 to 11ff389 Compare April 7, 2025 12:08
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was not migrated to JUnit5.

While it would be possible the combination of JenkinsConfiguredWithCodeRule and Envs / Env / EnvsFromFile makes it rather tricky and would require further changes to the tests. As I can not be sure that this would not change the intent of a test, I decided to leave it out of the migration.

* Migrate annotations and imports
* Migrate assertions
* Remove public visibility for test classes and methods
* Minor code cleanup
@strangelookingnerd
Copy link
Contributor Author

@jenkinsci/hashicorp-vault-plugin-developers Kindly requesting a review.

@dineshba dineshba requested a review from Copilot June 26, 2025 11:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the existing test suite from JUnit4 to JUnit5, updates related imports and annotations, and cleans up test utility classes.

  • Converted test classes and lifecycle methods to JUnit Jupiter annotations
  • Replaced interface-based constant usage with static imports in utility classes
  • Updated project POM to include JUnit Jupiter and Testcontainers JUnit support

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
VaultTestUtil.java Updated VaultContainer generics and replaced interface-based constants with static imports
VaultContainer.java Updated Javadoc for lifecycle hook and replaced deprecated Testcontainers API calls
TestConstants.java Centralized new constants for JUnit5 tests
MaskingConsoleLogFilterSpec.java Switched to JUnit5 with MockitoExtension and updated annotations
VaultSecretSourceTest.java Partial JUnit5 migration; still contains JUnit4 rules and lifecycle annotations
pom.xml Added Testcontainers JUnit Jupiter dependency and switched Mockito to JUnit Jupiter
Comments suppressed due to low confidence (1)

Copy link
Contributor

@dineshba dineshba left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @strangelookingnerd. LGTM

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.

2 participants