-
Notifications
You must be signed in to change notification settings - Fork 729
[UNIT]tests: add wasm-c-api unit tests #4677
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
base: dev/ai_generated_cases
Are you sure you want to change the base?
[UNIT]tests: add wasm-c-api unit tests #4677
Conversation
Add enhanced test coverage for wasm-c-api functionality: - Add wasm_extern_type testing with null input validation - Add wasm_memory internal API tests with WebAssembly module integration - Extend CMakeLists.txt to include new test files Signed-off-by: Gong, Pu <[email protected]>
1be7026 to
328b14b
Compare
lum1n0us
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/unit/wasm-c-api/enhanced_gen_wasm_c_api_test.cc is better than tests/unit/wasm-c-api/enhanced_gen_wasm_memory_new_internal_test.cc.
| ASSERT_NE(nullptr, engine); | ||
| store = wasm_store_new(engine); | ||
| ASSERT_NE(nullptr, store); | ||
| runtime = std::make_unique<WAMRRuntimeRAII<512 * 1024>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not call both wasm_engine_new() and wasm_runtime_full_init()(via WAMRRuntimeRAII). In this case, WAMRRuntimeRAII should not be involved.
|
|
||
| // Cleanup | ||
| wasm_func_delete(func); | ||
| wasm_functype_delete(functype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to cleanup externtype.
The own in its declaration WASM_API_EXTERN own wasm_externtype_t* wasm_extern_type(const wasm_extern_t*); indicates an ownership transfer.
|
|
||
| // Cleanup | ||
| wasm_func_delete(func); | ||
| wasm_functype_delete(functype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to cleanup externtype.
The own in its declaration WASM_API_EXTERN own wasm_externtype_t* wasm_extern_type(const wasm_extern_t*); indicates an ownership transfer.
|
|
||
| // Cleanup | ||
| wasm_global_delete(global); | ||
| wasm_globaltype_delete(globaltype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to cleanup externtype.
The own in its declaration WASM_API_EXTERN own wasm_externtype_t* wasm_extern_type(const wasm_extern_t*); indicates an ownership transfer.
| // The fact that we can get exports means the instance was created | ||
| // successfully which means wasm_memory_new_internal was called and | ||
| // succeeded | ||
| ASSERT_GE(exports.size, 0u); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be 0.
| } | ||
|
|
||
| // Create multiple instances to exercise memory creation paths | ||
| for (int i = 0; i < 3; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 3 times.
|
|
||
| // Test Case 12: Test error handling paths in memory creation | ||
| TEST_F(WasmMemoryNewInternalTest, | ||
| ErrorHandlingPaths_MemoryCreation_HandlesFailuresGracefully) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to a previous one.
|
|
||
| // Test Case 13: Test comprehensive parameter validation | ||
| TEST_F(WasmMemoryNewInternalTest, | ||
| ParameterValidation_ComprehensiveTest_CoversAllPaths) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case appears to be a simple combination of previous ones.
tests/unit/wasm-c-api/enhanced_gen_wasm_c_api_test.ccPASS: 11. NEED MODIFICATION: 1. DROP: 1.
tests/unit/wasm-c-api/enhanced_gen_wasm_memory_new_internal_test.ccPASS: 4. NEED MODIFICATION: 3. DROP: 6.
|
Add enhanced test coverage for wasm-c-api functionality: