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 AstCheckerTest to Junit5 #3960

Merged
merged 1 commit into from
May 26, 2021
Merged

test: Migrate AstCheckerTest to Junit5 #3960

merged 1 commit into from
May 26, 2021

Conversation

Rohitesh-Kumar-Jain
Copy link
Contributor

#3919

This PR just migrates AstCheckerTest to Junit5, pure refactoring.

@Rohitesh-Kumar-Jain
Copy link
Contributor Author

Hi migrating AstCheckerTest to Junit5 involved only 2 changes, I was wondering that should I raise PRs that are this small 😅, or should prioritize bigger tests first instead of going sequentially?

@slarse
Copy link
Collaborator

slarse commented May 26, 2021

@Rohitesh-Kumar-Jain Yes, your recent changes have made it clear that these migrations are pretty tiny. Let's try to bunch them up a bit.

Let's try something semi-extreme: try to migrate every test class in the the spoon.reflect that does not require any extensive changes (i.e. at most swap the order of arguments to some assertion). Keep going until you've a total of ~100 changes (insertions + deletions).

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.

This one is also fine, looks like @Rohitesh-Kumar-Jain is starting to get comfortable with submitting PRs :)

@monperrus monperrus changed the title review: test: Migrate AstCheckerTest to Junit5 test: Migrate AstCheckerTest to Junit5 May 26, 2021
@monperrus monperrus merged commit 65e4bf0 into INRIA:master May 26, 2021
@Rohitesh-Kumar-Jain
Copy link
Contributor Author

@Rohitesh-Kumar-Jain Yes, your recent changes have made it clear that these migrations are pretty tiny. Let's try to bunch them up a bit.

Let's try something semi-extreme: try to migrate every test class in the the spoon.reflect that does not require any extensive changes (i.e. at most swap the order of arguments to some assertion). Keep going until you've a total of ~100 changes (insertions + deletions).

Yes, this seems a good idea and for the test involving many changes, I will raise PRs for them separately.

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain deleted the migrate-astcheckertest-to-junit5 branch May 26, 2021 15:52
@monperrus monperrus mentioned this pull request Aug 19, 2021
woutersmeenk pushed a commit to woutersmeenk/spoon that referenced this pull request Aug 29, 2021
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