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_fault_handler: exit nonzero if expected segfault missing #378

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

fw-immunant
Copy link
Contributor

We previously tested for the printed message with FileCheck, but now our Criterion-based tests only care about exit codes.

This was hiding test failures on both AArch64 and x86_64.

Based on bisecting it appears the three_keys_minimal test has never worked but its failures have always been masked by the test_fault_handler bug that this fixes. I'll look into things more tomorrow.

Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

This makes sense to me from what I can tell.

Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Did you also want to disable three_keys_minimal in CI until it's fixed?

@fw-immunant
Copy link
Contributor Author

I think we can wait to merge this until we figure out what's wrong with that test and heap_two_keys.

@rinon
Copy link
Collaborator

rinon commented Oct 3, 2024

Do we have an issue on three_keys_minimal and heap_two_keys? If we're not working on that currently I think we should disable those tests and land this PR.

@rinon
Copy link
Collaborator

rinon commented Oct 3, 2024

It looks like we need to delete the build directory in CI? #418

@rinon rinon force-pushed the fw/should-segfault branch from 5a27f6c to d0fefda Compare October 3, 2024 23:10
fw-immunant and others added 4 commits October 3, 2024 16:36
we previously tested for the printed message with FileCheck, but now our criterion-based tests care about exit codes

this was hiding test failures on ARM64
@rinon rinon force-pushed the fw/should-segfault branch from d0fefda to a94708b Compare October 3, 2024 23:36
@rinon rinon merged commit deeab9a into main Oct 4, 2024
34 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.

3 participants