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

test: migrate CtTypeReferenceTest to Junit5 #3969

Merged
merged 3 commits into from
Jun 1, 2021
Merged

test: migrate CtTypeReferenceTest to Junit5 #3969

merged 3 commits into from
Jun 1, 2021

Conversation

Rohitesh-Kumar-Jain
Copy link
Contributor

#3919

This PR migrates FilterTest to Junit5.

Hi @slarse,

I tried to replaceRule with ExtendWith, but wasn't able to replace it successfully.

I would be glad if someone may help me perform this replacement.

I am unable to understand MockitoExtension isn't getting resolved despite adding dependencies. (references : baeldung & stackoverflow).

@slarse
Copy link
Collaborator

slarse commented May 31, 2021

Hi @Rohitesh-Kumar-Jain,

There's no problem resolving the Mockito extension. The error message here says that there are unnecessary stubbings (i.e. some methods are being stubbed but aren't used in the test case): https://github.com/INRIA/spoon/pull/3969/checks?check_run_id=2701008263#step:10:452

There are two stubbings that are uneccessary for one test (but necessary for the other), the error message says exactly which lines correspond to the unnecessary stubbings. Since these stubbings already existed as-is, I would make them lenient (see the baeldung article you linked) to avoid having to redesign the test case.

When you get a failure in GitHub Actions, click the "Details" button and it will take you to the error. You may have to scroll up and down to actually find the source of the error. Note that a test failure can't occur if there are failures to resolve dependencies, as in that case the project won't even compile (there'll be a compile error).

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain changed the title help: test: migrate CtTypeReferenceTest to Junit5 review: test: migrate CtTypeReferenceTest to Junit5 Jun 1, 2021
@Rohitesh-Kumar-Jain
Copy link
Contributor Author

@slarse thanks for helping me making this PR become green : )

Copy link
Collaborator

@slarse slarse left a comment

Choose a reason for hiding this comment

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

Just revert an unrelated whitespace change and then we're good!

In general, try to avoid changing stuff outside of the scope of your PR. It's a good thing to look over your changes one extra time when you consider yourself done :)

spoon-pom/pom.xml Outdated Show resolved Hide resolved
Co-authored-by: Simon Larsén <slarse@kth.se>
@Rohitesh-Kumar-Jain
Copy link
Contributor Author

In general, try to avoid changing stuff outside of the scope of your PR. It's a good thing to look over your changes one extra time when you consider yourself done :)

Thanks, I will keep this in my mind for future PRs

Copy link
Collaborator

@slarse slarse left a comment

Choose a reason for hiding this comment

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

@Rohitesh-Kumar-Jain LGTM, nice work! I suggest you try to get started with writing some tests now, perhaps in one of the test files you've converted. It seems to me like you have no problems with the general flow of a pull request anymore.

@monperrus ping for merge

@monperrus monperrus changed the title review: test: migrate CtTypeReferenceTest to Junit5 test: migrate CtTypeReferenceTest to Junit5 Jun 1, 2021
@monperrus monperrus merged commit ba184e7 into INRIA:master Jun 1, 2021
@monperrus
Copy link
Collaborator

🚀 @Rohitesh-Kumar-Jain and thanks 🥇 @slarse for the great onboarding

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain deleted the migrate-cttypereferencetest-to-junit5 branch June 1, 2021 12:02
@monperrus monperrus mentioned this pull request Aug 19, 2021
woutersmeenk pushed a commit to woutersmeenk/spoon that referenced this pull request Aug 29, 2021
Co-authored-by: Simon Larsén <slarse@kth.se>
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.

3 participants
  NODES
COMMUNITY 2
Note 1
Project 4
USERS 1