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 contextSize test parameter to enlarge the scope of a diff #788

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TonioGela
Copy link

As per description, this PR introduces a contextSize overridable suite parameter that control the number of lines around the reported diff in case of a mismatch ( ala grep -A <n> -B <n>)

Ofc the normal behaviour was kept identical to the actual, and there's no change in the user API (at least for the methods used via Assertions)

@TonioGela
Copy link
Author

Hmm, given that a lot of the stuff in Diff is effectively exposed the mima check screams with 11 compatibility errors. I'll try to figure out a better encoding for the content of this PR, so don't take it too seriously for the moment 😇

@tgodzik
Copy link
Contributor

tgodzik commented May 28, 2024

Shouldn't this be a parameter to the runner

?

@TonioGela
Copy link
Author

Shouldn't this be a parameter to the runner


?

Interesting question. Going further: should it just be a parameter or also a parameter?

@tgodzik
Copy link
Contributor

tgodzik commented May 28, 2024

I think just a parameter since it doesn't really change the test logic, but how the test is being displayed. Potentially we could have runner defaults for a specific suite as a separate thing.

@tgodzik
Copy link
Contributor

tgodzik commented May 28, 2024

btw. MiMa is not a huge issue here, we would need to do 1.1.x next so that it's automatically picked up as possible incompatibility

@TonioGela
Copy link
Author

I think just a parameter since it doesn't really change the test logic, but how the test is being displayed. Potentially we could have runner defaults for a specific suite as a separate thing.

Okay, this seems reasonable. I'll try to rework it. I'll also try to add documentation about that flag maybe here: https://scalameta.org/munit/docs/troubleshooting.html

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