-
-
Notifications
You must be signed in to change notification settings - Fork 670
Small fixes for tests and CI #2722
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
Merged
Merged
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
127aa8c
to
f23adab
Compare
f23adab
to
354ee74
Compare
dcodeIO
reviewed
Jul 21, 2023
HerrCai0907
reviewed
Jul 21, 2023
f2eae37
to
6cd99a9
Compare
I just changed the functionality of |
This change makes ASC_FEATURES only affect whether the instantiate step is executed, and not whether tests are compiled and fixtures checked.
Although the previous commit ensures the fixtures match for all tests requiring ASC_FEATURES, this commit ensures these tests can be instantiated. The list of tests to check will (for now) remain a whitelist, but since the fixtures of *all* tests are checked by CI, the PR breaking any excluded test can be found via `git blame` or `git log`.
Although the rest of the compiler switched to Pascal case, this small test didn't. This equally small fix makes the test work again (instead of spamming "Identifier").
6cd99a9
to
a726f89
Compare
75d6e1e
to
6a2b5cc
Compare
@dcodeIO @HerrCai0907 This PR should be fully ready as far as I know. |
This commit changes compiler tests such that WAT fixtures are compared in release builds as well. Moreover, the JS and (D)TS bindings are now compared as well, for both debug and release builds. These changes should ensure changes to Binaryen or bindings generation that affect the release WAT or JS/TS bindings respectively must update the broken compiler tests as well.
6a2b5cc
to
77aad42
Compare
HerrCai0907
approved these changes
Jul 26, 2023
No longer shall VS Code cry about interface I in class-implements.ts already being declared in infer-type.ts and static-array.ts! By the power of forcing ESM detection!
dcodeIO
approved these changes
Jul 31, 2023
Okay, @MaxGraey, what do you think? |
MaxGraey
approved these changes
Jul 31, 2023
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.
Changes proposed in this pull request:
⯈ Ensure all tests are run in CI, including those involving experimental features
⯈ Fix the tokenizer test, as useless as it may be
⯈ Ensure the release fixtures and bindings fixtures are checked
⯈ Fix the typings for the
writeFile
CLI option, since strings are passed in too⯈ Make VS Code complain less about variables in tests being redeclared in other files
Closes #2723.