Added support for purposeful failing test cases #528
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
End-to-End Test ReportTest PreviewTest Report Deterministic TestsSummary
Test Results by Category
Non Deterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
|
you need to change the path of signal_resethand so its still skipped now that its moved. We'll have someone else look into whats going on there. |
m-hemmings
previously approved these changes
Nov 17, 2025
Member
m-hemmings
left a comment
m-hemmings
left a comment
There was a problem hiding this comment.
Most of my comments are cosmetic, looks good except for a few nits
| native_success, native_output, native_retcode, native_error = compile_and_run_native(source_file, timeout_sec) | ||
|
|
||
| # If native compile failed, report and return | ||
| if native_error == "Failure_native_compiling": |
Member
There was a problem hiding this comment.
Nice early-abort. One thought: this means a fail-test doesn't expect compile errors. Fine, but maybe worth a note since it’s distinct from “expected failure = runtime failure,” not “compile must fail.”
scripts/wasmtestreport.py
Outdated
| return | ||
|
|
||
| # Compile and run WASM | ||
| wasm_file, compile_err = compile_c_to_wasm(source_file) |
Member
There was a problem hiding this comment.
Clean. One tiny improvement: you might want to name the error more explicitly later (e.g., wasm_compile_error) to keep symmetry with native.
| if isinstance(wasm_retcode, str): | ||
| wasm_failed = True # timeout or unknown_error means it failed | ||
| else: | ||
| wasm_failed = wasm_retcode != 0 |
Member
There was a problem hiding this comment.
This works, but I’d tighten it up in case later on someone forgets why string return codes mean “failed.”
scripts/wasmtestreport.py
Outdated
| handler.add_success(output_info) | ||
| elif not native_failed and not wasm_failed: | ||
| # Both succeeded when they should have failed | ||
| failure_info = ( |
Member
There was a problem hiding this comment.
Only suggestion: the block that builds failure_info is duplicated 3× here . A helper would de-duplicate and avoid future formatting drift.
scripts/wasmtestreport.py
Outdated
| "Output_mismatch": "C Compiler and Wasm Output mismatch", | ||
| "Fail_native_succeeded": "Fail Test: Native Succeeded (Should Fail)", | ||
| "Fail_wasm_succeeded": "Fail Test: Wasm Succeeded (Should Fail)", | ||
| "Fail_both_succeeded": "Fail Test: Both Native and Wasm Succeeded (Should Fail)" |
Member
There was a problem hiding this comment.
Nice, explicit, readable. Would be even nicer if you grouped these together with a comment like:
Expected-failure test modes
Contributor
|
Looks good. As Nick mentioned, just change the path of signal_resethand test in the skip list since it is failing. |
Contributor
End-to-End Test ReportTest PreviewTest Report Deterministic TestsSummary
Test Results by Category
Non Deterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
drapl0n
approved these changes
Nov 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test suite now supports purposeful failing tests:
New tests that now pass:
mmap-negative1.c
mmap-negative2.c