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.

Sign up for GitHub

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

Jump to bottom

Official Testsuite Runner #112

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Official Testsuite Runner #112

wants to merge 4 commits into from
+1,408 −182

Conversation

nerodesu017
Copy link
Collaborator

@nerodesu017 nerodesu017 commented Dec 16, 2024 •
edited
Loading

Pull Request Overview

This is an on-going PR focused on making the spec test runner work again + add more to it.
Additions:

  • filtering (either Include or Exclude) based on files and folders (so we can discard the proposals folder, for example, or files related to SIMD, which we do not implement (yet), and that give us a rough time)
  • assert_trap being implemented
  • a few debug (with debug_assertions) related code for stringifying errors and other features for QoL when debugging
  • changes to how we cast certain values (especially F64 and F32) so that we get the desired results (mostly bit reinterpretation, not normal casting, this is in part because the crate that we use for parsing WAST files parses these values into u32 and u64, respectively, and we have to reinterpret the bits - see Future bug: Potential float alteration on some architectures #28 for more info)
  • a fix for a Span that we return when reading a constant expression (for example, for a global) because we were returning 1 more byte, this wouldn't be a problem, as we stop on the END instruction, but it is a problem when the END is EOF and we return +1 byte and we throw an error when doing a later check - see src/validation/read_constant_expression.rs

See commit message for more!

Testing Strategy

This pull request was tested by... N/A

TODO or Help Wanted

This pull request still needs... N/A

Formatting

  • Ran cargo fmt
  • Ran cargo check
  • Ran cargo build
  • Ran cargo doc
  • Ran nix fmt

Github Issue

This pull request closes <GITHUB_ISSUE>

@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 5 times, most recently from 14f2dd5 to 5d776f7 Compare December 17, 2024 13:24
@codecov Codecov
Copy link

codecov bot commented Dec 17, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 90.90909% with 41 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/core/error.rs 0.00% 11 Missing ⚠️
src/execution/mod.rs 87.64% 5 Missing and 6 partials ⚠️
src/validation/code.rs 76.74% 10 Missing ⚠️
src/validation/mod.rs 76.00% 5 Missing and 1 partial ⚠️
src/core/reader/types/import.rs 0.00% 0 Missing and 2 partials ⚠️
src/core/reader/types/values.rs 95.23% 0 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
src/core/reader/mod.rs 99.02% <ø> (+0.48%) ⬆️
src/core/reader/types/memarg.rs 100.00% <100.00%> (+20.00%) ⬆️
src/core/reader/types/opcode.rs 100.00% <100.00%> (ø)
src/core/utils.rs 100.00% <100.00%> (ø)
src/execution/function_ref.rs 53.33% <ø> (ø)
src/execution/interpreter_loop.rs 99.08% <100.00%> (+6.83%) ⬆️
src/execution/value.rs 90.11% <100.00%> (+5.71%) ⬆️
src/execution/value_stack.rs 92.70% <100.00%> (+8.87%) ⬆️
src/validation/read_constant_expression.rs 74.35% <100.00%> (+12.82%) ⬆️
src/core/reader/types/values.rs 91.33% <95.23%> (+8.00%) ⬆️
... and 5 more

... and 11 files with indirect coverage changes

@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 3 times, most recently from d8e21d6 to 750e5b6 Compare January 13, 2025 15:47
@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 4 times, most recently from 8def0b8 to 42dda9b Compare January 20, 2025 14:02
@nerodesu017 nerodesu017 self-assigned this Jan 20, 2025
@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch from 6fd926c to b7f554e Compare January 27, 2025 13:22
@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 2 times, most recently from 0cb7ed2 to 6b4f96d Compare January 27, 2025 13:47
@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 5 times, most recently from 1982d04 to bb8e9f0 Compare January 27, 2025 15:17
@nerodesu017
Copy link
Collaborator Author

Can't use harness with [[test]] in Cargo.toml because nix action will whine about output.
Also, can't really upload them anywhere because github actions do NOT want to work with me here and it's kind of painful

@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch 5 times, most recently from 9f94765 to 50f647d Compare January 27, 2025 16:10
cemonem
cemonem previously approved these changes Feb 3, 2025
View reviewed changes
george-cosma
george-cosma previously approved these changes Feb 3, 2025
View reviewed changes
src/core/reader/types/values.rs Show resolved Hide resolved
src/core/reader/types/opcode.rs Show resolved Hide resolved
@nerodesu017 nerodesu017 added this pull request to the merge queue Feb 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Feb 3, 2025
@nerodesu017
feat: testsuite runner update
703ea24 
CHANGELOG
- fixed testsuite runner panicking
- fixed wrong Span length
- fixed debug assertions on import (moved to using full name on function call)
- removed invoke_dynamic_unchecked_return_ty from debug_assertions
- added runtime error stringification to testsuite runner
- assert_malformed implementation
- fixed locals validation check
- added the support for testing singular files in the wasm testsuite (for easier debugging)
- added support for assert_invalid + lonely invoke
- modified reports' output for unknown line number
- added check for when there are multiple memories (along with the imported memories)
- fixed memarg alignment
- fixed check for memory instances
- added copyright notice
- implemented correct assert_eq for floats
- catch_unwind instantiation (as it can panic) and impl Error for our Error types
- fixed float operations
- fixed memory.fill operation
- moved more panics and asserts to errors
- README for test runner
- fixed memarg.align checks
- removed the display of stacktraces from panics
- added display of tests in a decreasing order based on the percentage of inner tests that pass
- fixed MemArg::read so that we correctly read the offset (it can be a u64, and we have to be careful of overflowing)
- made read_var_u32 depend on read_var_u64 so we catch all overflows
- fixed float operations: ceil, floor, trunc, nearest, min, max
- removed wrong tests in f32.rs and f64.rs
- better test results display

Signed-off-by: nerodesu017 <[email protected]>
@nerodesu017
feat: basic support for custom sections
b0bfc4c 
Signed-off-by: nerodesu017 <[email protected]>
@nerodesu017
chore: make test crash on uninitialized submodules; make testrunner o…
43cc864 
…utput to console

Signed-off-by: nerodesu017 <[email protected]>
@nerodesu017
chore: see output of tests
f3cf3dc 
Signed-off-by: nerodesu017 <[email protected]>
@nerodesu017 nerodesu017 dismissed stale reviews from george-cosma and cemonem via f3cf3dc February 3, 2025 13:00
@nerodesu017 nerodesu017 force-pushed the nero/dev/testsuite-runner branch from 50f647d to f3cf3dc Compare February 3, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cemonem cemonem cemonem approved these changes

@wucke13 wucke13 Awaiting requested review from wucke13

@george-cosma george-cosma Awaiting requested review from george-cosma

At least 2 approving reviews are required to merge this pull request.

Assignees

@nerodesu017 nerodesu017

Labels
core execution priority-high priority-medium tests validation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nerodesu017 @cemonem @george-cosma