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] Move basic tests in lit/ to lit/basic/ #6156

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Dec 8, 2023

Here 'basic' tests means that what we have in binaryen/test/. We check three things with those tests:

  • Run wasm-opt -all -g on it and compare the output with *.from-wast
  • Run wasm-as -all -g and wasm-dis on it and compare the output with *.fromBinary.
  • Run wasm-as -all and wasm-dis on it and compare the output with *.fromBinary.noDebugInfo.

I planned to move those to test/lit/. But test/lit/ has other kind of tests as well, so I think it'd be nice to have a dedicated directory for these tests.

Before doing that, I noticed there are already four tests that have been already ported to do this, and this PR moves them to test/lit/basic/.

I couldn't come up with a better name than basic. If you have other suggestions please let me know.

Here 'basic' tests means that what we have in binaryen/test/. We checked
three things with those tests:
- Run `wasm-opt -all -g` on it and compare the output with `*.from-wast`
- Run `wasm-as -all -g` and `wasm-dis` on it and compare the output with
  `*.fromBinary`.
- Run `wasm-as -all` and `wasm-dis` on it and compare the output with
  `*.fromBinary.noDebugInfo`.

I plan to move those to `test/lit/`. But `test/lit/` has other kind of
tests as well, so I think it'd be nice to have a dedicated directory for
these tests.

Before doing that, I noticed there are already four tests that have been
already ported to do this, and this PR moves them to `test/lit/basic/`.

I couldn't come up with a better name than `basic`. If you have other
suggestions please let me know.
@tlively
Copy link
Member

tlively commented Dec 8, 2023

Moving these to a basic directory sounds good to me!

The CHECK-NODEBUG output appears at the end because the auto-update script doesn't know how to match up the minimal generated names with the human-readable input names.

@aheejin
Copy link
Member Author

aheejin commented Dec 8, 2023

The CHECK-NODEBUG output appears at the end because the auto-update script doesn't know how to match up the minimal generated names with the human-readable input names.

Yeah I realized that and deleted the comment but it was too late 😅
But by the way, I'm trying to port tests in test/ into lit, but some files just don't generate NODEBUG lines. For example:

;; RUN: wasm-as %s -all -o %t.wasm                                               
;; RUN: wasm-dis %t.wasm -all -o %t.bin.wast                                     
;; RUN: cat %t.bin.wast | filecheck %s --check-prefix=CHECK                      
                                                                                 
(module                                                                          
 (func $test                                                                     
  (drop                                                                          
    (i32.const 0)                                                                
  )                                                                              
 )                                                                               
)

Note that there's no -g in the wasm-as line. When I run update_lit_checks.py, CHECK lines don't get generated at the end; they are just not generated. Do you know why?

@tlively
Copy link
Member

tlively commented Dec 8, 2023

Do the tests with the missing checks not pass --all-items to the update script? That’s the only thing I can think of.

@aheejin
Copy link
Member Author

aheejin commented Dec 8, 2023

Do the tests with the missing checks not pass --all-items to the update script? That’s the only thing I can think of.

Huh, yeah, passing --all-items seems to do the trick. But wasn't that for generating other module items such as type or memory? I thought funcs should be generated without the flag.

@aheejin aheejin merged commit 48373b6 into WebAssembly:main Dec 8, 2023
14 checks passed
@aheejin aheejin deleted the move_test_basic branch December 8, 2023 23:01
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
Here 'basic' tests means that what we have in `binaryen/test/`. We checked
three things with those tests:
- Run `wasm-opt -all -g` on it and compare the output with `*.from-wast`
- Run `wasm-as -all -g` and `wasm-dis` on it and compare the output with
  `*.fromBinary`.
- Run `wasm-as -all` and `wasm-dis` on it and compare the output with
  `*.fromBinary.noDebugInfo`.

I planned to move those to `test/lit/`. But `test/lit/` has other kind of
tests as well, so I think it'd be nice to have a dedicated directory for
these tests.

Before doing that, I noticed there are already four tests that have been
already ported to do this, and this PR moves them to `test/lit/basic/`.

I couldn't come up with a better name than `basic`. If you have other
suggestions please let me know.
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