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

Continue on fail when max_fail is gt 0 #332

Merged
merged 4 commits into from
Mar 5, 2025
Merged

Continue on fail when max_fail is gt 0 #332

merged 4 commits into from
Mar 5, 2025

Conversation

Pat-Lafon
Copy link
Contributor

I think I ran into #182, where I want to set max_fail to be some value greater than 1.

In my limited sleuthing, I believe the check could have been accidentally removed in 3a9bf2e#diff-07dbdff6f84a44f565ad4871c5790ad598ceda123f0b8190a711d303d3be28efR1718-R1720 (Hmm, it's highlighted in the link, but it doesn't automatically go to the corresponding line number which is 1767 in file src/core/QCheck2.ml). Here, it use to be that if some small parameter and state.cur_max_fail > 0, then it would continue otherwise yield. I've just added that latter back in. In the current codebase, it doesn't seem like cur_max_fail does anything(4 matches for searching this keyword).

Besides just running this locally and seeing that in my example I can report multiple failures in the failure column, I haven't looked into how to test this. I also haven't confirmed is this actually solves #182 other than seeing that the suspected commit was in April 2021 and the issue was filed in September 2021

@jmid jmid linked an issue Mar 5, 2025 that may be closed by this pull request
@jmid
Copy link
Collaborator

jmid commented Mar 5, 2025

Thanks a bunch for digging into this issue and suggesting a fix 🙏 🙂

I've added a couple of tests, filed updated expect test outputs for 32+64 bit builds, and added a CHANGELOG entry.

@Pat-Lafon
Copy link
Contributor Author

Thanks for adding tests! All seems good to me.

@jmid
Copy link
Collaborator

jmid commented Mar 5, 2025

Cool, thanks again 🙂

@jmid jmid merged commit 7125521 into c-cube:main Mar 5, 2025
13 checks passed
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.

0.18 max_fail regression
2 participants