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

Comprehensive test for interpreter, new global input type info tests, support match on all types, restricted global variables and other fixes #978

Merged
merged 50 commits into from
Oct 2, 2024

Conversation

afsalthaj
Copy link
Contributor

@afsalthaj afsalthaj commented Sep 30, 2024

Fixes #876
Fixes #881
Fixes #954

  • Added a comprehensive test in rib interpreter module replacing random tests that existed in worker-service module. This is a complete replacement and not migration. Tests in worker-service turned out to be overly repetitive and deleted (almost deleting 2000 lines of code including some support functions). The new comprehensive test is just 1 test case but it covers almost all possibilities of function inputs and function outputs in terms of types. It's another 2000 lines, however, more structured. This test exist along with other fine grained 400 tests in Rib.

  • Added tests specifically covering the scenarios of global-inputs in compiler module.

  • Support pattern matching on all types

  • Restricted Global Variables in Rib, that worker service can satisfy only request variable when using Rib.

  • Other minor fixes

@afsalthaj afsalthaj changed the title Migrate tests Add global input type info tests Migrate tests ,Add global input type info tests, and fix other issues Sep 30, 2024
@afsalthaj afsalthaj changed the title Migrate tests ,Add global input type info tests, and fix other issues Migrate tests ,Add global input type info tests, support match on all types, and other fixes Sep 30, 2024
@afsalthaj
Copy link
Contributor Author

It was difficult to satisfy wasm-wave syntax here: https://github.com/golemcloud/golem/blob/migrate_tests/golem-rib/src/interpreter/comprehensive_test/mock_data.rs#L155, mainly due to abstract error messages and not mentioning the exact reason: invalid token at 31..32.

If we are using the error message of https://github.com/golemcloud/wasm-rpc/blob/main/wasm-rpc/src/text.rs#L25 (which in turn comes from wasm-wave crate) back to users, then we need to be careful. Hopefully they will improving since it got migrated.

@afsalthaj afsalthaj changed the title Migrate tests ,Add global input type info tests, support match on all types, and other fixes Comprehensive test for interpreter, new global input type info tests, support match on all types, restricted global variables and other fixes Oct 1, 2024
@afsalthaj afsalthaj marked this pull request as ready for review October 1, 2024 12:16
@afsalthaj afsalthaj merged commit 033576f into main Oct 2, 2024
17 checks passed
@afsalthaj afsalthaj deleted the migrate_tests branch October 2, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants