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

Add unit tests for JMockit Delegate to mockito migration. Also add test case for comments #557

Merged
merged 6 commits into from
Aug 4, 2024

Conversation

shivanisky
Copy link
Collaborator

@shivanisky shivanisky commented Jul 28, 2024

At the moment, JMockit Delegates are not migrated to mockito as mentioned in issue

What I'm seeing is that they are being trashed with the template being printed out. These tests were written to try to replicate this issue, however I was unable to.
They may help anyone adding the feature for Delegate migration.

https://javadoc.io/doc/org.jmockit/jmockit/latest/mockit/Delegate.html

Also added one test case showing comments are not preserved.

I am wondering if it's better to have the test cases failing with the expected output and to disable them. Please let me know if that's preferred.

Anyone you would like to review specifically?

@timtebeek @tinder-dthomson

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

…e test case for comments for JMockit Expectations proving that comments are not preserved.
@timtebeek
Copy link
Contributor

Thanks for the continued work on this! Indeed probably best to have the tests fail with the actual expected output, as opposed to documenting the status quo. No need to mark them @Disabled just yet, we can do that just before a merge if we want to delay any work towards this, and then have the tests reference an open issue with more information.

@timtebeek timtebeek marked this pull request as draft July 29, 2024 14:38
Modify test cases so that the tests fail with expected output of Delegate migration
Modify test to prove that comments migration is not supported within expectations, and this may be useful for someone who is adding this feature later. Disable test.
timtebeek and others added 2 commits August 3, 2024 19:46
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Modify tests to account for stubbing void methods using doAnswer instead of thenAnswer. Remove unnecessary imports
@shivanisky shivanisky marked this pull request as ready for review August 4, 2024 01:44
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Great to have these examples documented and runnable! One last question before we merge, but other than that good to go.

@timtebeek timtebeek added documentation Improvements or additions to documentation recipe Recipe request labels Aug 4, 2024
@timtebeek timtebeek merged commit 533f37d into openrewrite:main Aug 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants