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] Make get_tests return only files #6164

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Dec 10, 2023

Currently get_tests returns files and directories, especially when the extension is not given. This makes get_tests return a directory like test/wasm2js/ as a test.

wasm2js.py's check_for_stale_files errors out when there are files within test/wasm2js/ whose basenames don't match any files within any of test/, test/spec/, test/wasm2js/.

def check_for_stale_files():
if shared.options.test_name_filter:
return
# TODO(sbc): Generalize and apply other test suites
all_tests = []
for t in tests + spec_tests + wasm2js_tests:
all_tests.append(os.path.basename(os.path.splitext(t)[0]))
all_files = os.listdir(shared.get_test_dir('wasm2js'))
for f in all_files:
prefix = f.split('.')[0]
if prefix not in all_tests:
shared.fail_with_error('orphan test output: %s' % f)

wasm2js.wast.asserts is apparently a special case for asserts test:

assert_tests = ['wasm2js.wast.asserts']
and this doesn't seem to have the matching wast tests in the three test directories. But it just happened to not error out because get_tests returns directory names too and one of them was wasm2js (test/wasm2js/ directory).

This makes get_tests return only files, and make files in assert_tests not error out additionally.

Currently `get_tests` returns files and directories, especially when
the extension is not given. This makes `get_tests` return a directory
like `test/wasm2js/` as a test.

`wasm2js.py`'s `check_for_stale_files` errors out when there are files
within `test/wasm2js/` whose basenames don't match any files within any
of `test/`, `test/spec/`, `test/wasm2js/`.
https://github.com/WebAssembly/binaryen/blob/1d615b38dd4152494d2f4d3520c8b1d917624a30/scripts/test/wasm2js.py#L33-L46

`wasm2js.wast.asserts` is apparently a special case for asserts test:
https://github.com/WebAssembly/binaryen/blob/1d615b38dd4152494d2f4d3520c8b1d917624a30/scripts/test/wasm2js.py#L28
and this doesn't seem to have the matching `wast` tests in the three
test directories. But it just happened to not error out because
`get_tests` returns directory names too and one of them was `wasm2js`
(`test/wasm2js/` directory).

This makes `get_tests` return only files, and make files in
`assert_tests` not error out additionally.
@aheejin aheejin requested a review from sbc100 December 10, 2023 21:30
aheejin added a commit to aheejin/binaryen that referenced this pull request Dec 10, 2023
@aheejin aheejin merged commit 9e07c82 into WebAssembly:main Dec 12, 2023
14 checks passed
@aheejin aheejin deleted the get_tests_file branch December 12, 2023 01:40
aheejin added a commit to aheejin/binaryen that referenced this pull request Dec 12, 2023
I tried to exclude wasm2js asserts tests from `check_for_stale_files` in
 WebAssembly#6164, but ended up doing it incorrectly. The file I checked for was
`wasm2js.wast.asserts`, while the output I should have excluded was
`wasm2js.asserts.js`. This fixes the code so we now check the prefix and
not the filename.

This is currently breaking the main branch, which was caused by me
landing multiple PRs at once. Sorry for the breakage. Landing this will
fix it.
aheejin added a commit that referenced this pull request Dec 12, 2023
I tried to exclude wasm2js asserts tests from `check_for_stale_files` in
 #6164, but ended up doing it incorrectly. The file I checked for was
`wasm2js.wast.asserts`, while the output I should have excluded was
`wasm2js.asserts.js`. This fixes the code so we now check the prefix and
not the filename.
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
Currently `get_tests` returns files and directories, especially when
the extension is not given. This makes `get_tests` return a directory
like `test/wasm2js/` as a test.

`wasm2js.py`'s `check_for_stale_files` errors out when there are files
within `test/wasm2js/` whose basenames don't match any files within any
of `test/`, `test/spec/`, `test/wasm2js/`.
https://github.com/WebAssembly/binaryen/blob/1d615b38dd4152494d2f4d3520c8b1d917624a30/scripts/test/wasm2js.py#L33-L46

`wasm2js.wast.asserts` is apparently a special case for asserts test:
https://github.com/WebAssembly/binaryen/blob/1d615b38dd4152494d2f4d3520c8b1d917624a30/scripts/test/wasm2js.py#L28
and this doesn't seem to have the matching `wast` tests in the three
test directories. But it just happened to not error out because
`get_tests` returns directory names too and one of them was `wasm2js`
(`test/wasm2js/` directory).

This makes `get_tests` return only files, and make files in
`assert_tests` not error out additionally.
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
I tried to exclude wasm2js asserts tests from `check_for_stale_files` in
 WebAssembly#6164, but ended up doing it incorrectly. The file I checked for was
`wasm2js.wast.asserts`, while the output I should have excluded was
`wasm2js.asserts.js`. This fixes the code so we now check the prefix and
not the filename.
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