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

New failing test management using Jupiter extension #4696

Merged
merged 10 commits into from
Sep 19, 2024

Conversation

JooHyukKim
Copy link
Member

@JooHyukKim JooHyukKim commented Sep 12, 2024

This is from discussion regarding FasterXML/jackson#243 (reply in thread).

Descrption

jackson-databind seems like reasonable start since it is extended by most repos.

On approval will do the following for closure ...

  • move failing tests to where they belong(?) main /test dir
  • get rid of related maven config

Resources

@cowtowncoder
Copy link
Member

I think I like the idea of using annotation.

The one last thing I feel uneasy about is the location: while in some ways it is good to keep tests in their logical place (where they'd be if passing), it makes it more difficult to see the set of expected-to-fail cases. So...

  • We cannot leave tests under src/test/java/.../failing since Maven settings in parent pom.xml skip these (I mean, we could change it etc but easier not to try that)
  • But should there be something like src/test/java/.../tofix package? (sibling to existing "failing" package) Just a naming convention, no special handling, to contain test classes where at least some test methods are expected to fail?

WDYT?

I just like the ability to quickly see all tests that are for things-to-fix.

@JooHyukKim
Copy link
Member Author

JooHyukKim commented Sep 14, 2024

Yeah it makes sense since when any test start passing, we need to further take actions.

'tofix' would suffice 😆. Will work on the revision within today

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

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

Looks good -- but will probably wait until 2.19 branch started, after 2.18.0 release in a week or so.

@cowtowncoder
Copy link
Member

Re-thinking this one: probably want to do for 2.18. Just need to see how to get things merged best way.

@cowtowncoder
Copy link
Member

Merged 3 new classes separately, went cleanly. Now tackling renaming which may get messier.

@cowtowncoder cowtowncoder merged commit 462f0b5 into FasterXML:2.18 Sep 19, 2024
9 checks passed
@cowtowncoder
Copy link
Member

Phew! That merge resolution was quite something. But this is now fully merged, for 2.18 and 3.0!

@JooHyukKim
Copy link
Member Author

Phew! That merge resolution was quite something. But this is now fully merged, for 2.18 and 3.0!

Awww, thank u! 🙏🏽 Had I realized the work load, I would've done the merging into master.

Please lemme know next time hehe @cowtowncoder

@cowtowncoder
Copy link
Member

@JooHyukKim Sure, not a problem. But I was in the best position to do the merge since I had renamed couple of tests, knew 2 pass in 3.0 but not 2.x... lots of trivia needed :)
Those 2 cases were actually useful in validating that "passing fails annotated tests".

Also: I noticed there was one test (for issue #639 I think) duplicated so was able to delete that.

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