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

Consider all other exit codes besides 'test-success-exit-code' as failures #61

Closed
rodrigovalle opened this issue Jun 25, 2020 · 1 comment · Fixed by #65
Closed

Consider all other exit codes besides 'test-success-exit-code' as failures #61

rodrigovalle opened this issue Jun 25, 2020 · 1 comment · Fixed by #65
Labels
help wanted Extra attention is needed
Milestone

Comments

@rodrigovalle
Copy link

I was following along with Writing an OS in Rust post-06; after writing the stack_overflow() test I also tried testing the failure case. The posts suggests commenting out the .set_stack_index(DOUBLE_FAULT_IST_INDEX) line of the double-fault handler, so I did that and ran the test again.

The test doesn't fail, instead it enters a bootloop where the stack overflows, pushes the interrupt handler to the stack, triple-faults, and reboots QEMU. This is reproducible in the post-06 branch of phil-opp/blog_os as well.

I tried setting QEMU's -no-reboot option in test-args; this stops the bootlooping, but the test still passes because QEMU has a return code of 0 in this case. Bootimage passes this exit code on to cargo-test, which marks the test as passed.

I think it's a small change, but by considering all non test-success-exit-codes as test failures, we could set up tests that fail on any unexpected reboot of the system with -no-reboot. Successful runs would then only be reported by the test framework itself.

@rodrigovalle rodrigovalle changed the title Consider all other exit codes besides test-success-exit-code as failures Consider all other exit codes besides 'test-success-exit-code' as failures Jun 25, 2020
@phil-opp
Copy link
Member

Thanks a lot for reporting this! I fully agree and I'd be happy to merge a pull request for this. I think we should also set the --no-reboot argument by default for test executables, since bootloops are rarely what you want in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants