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

Migrate kernel and law tests to munit #3787

Draft
wants to merge 2 commits into
base: series/3.5.x
Choose a base branch
from

Conversation

armanbilge
Copy link
Member

This was the low-hanging fruit, just to dip our toe in the water.

Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

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

Solid start. Nothing in here needs changing, just using the Request Changes to hold the merge.

I think we should do two things. First, munit somewhat infamously circumvents sbt's output buffers for essentially no reason, and it occasionally causes weirdness with other tasks. I always have to look up how @etspaceman fixed it and cargo cult from him, since I can never remember it. Second, I'd prefer not to be in a state where we have two simultaneous test frameworks in use within the project, so I think it's worth driving this to completion and merging it all at once.

@armanbilge
Copy link
Member Author

First, munit somewhat infamously circumvents sbt's output buffers for essentially no reason, and it occasionally causes weirdness with other tasks.

Oh right that's extremely annoying. I've cargo-culted that too in various places, I'll make sure to bring that in.

Second, I'd prefer not to be in a state where we have two simultaneous test frameworks in use within the project, so I think it's worth driving this to completion and merging it all at once.

Fair enough!

@armanbilge armanbilge marked this pull request as draft September 2, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants