gemini-code-assist[bot] commented on code in PR #417:
URL: https://github.com/apache/tvm-ffi/pull/417#discussion_r2702479903


##########
tests/python/test_metadata.py:
##########
@@ -163,6 +163,16 @@ def test_metadata_global_func() -> None:
     assert metadata["str_attr"] == "hello"
 
 
+def test_metadata_empty_for_python_func() -> None:
+    @register_global_func("test.python_func_no_metadata")
+    def simple_func(x: int) -> int:
+        return x + 1
+
+    metadata = get_global_func_metadata("test.python_func_no_metadata")
+    assert metadata == {}
+    remove_global_func("test.python_func_no_metadata")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   This is a great regression test. To make it more robust and ensure that the 
global function is always removed even if the assertion fails, it's good 
practice to wrap the test logic in a `try...finally` block. This prevents test 
state from leaking and affecting other tests. Using a variable for the function 
name also improves maintainability by avoiding string literal repetition.
   
   ```suggestion
   def test_metadata_empty_for_python_func() -> None:
       func_name = "test.python_func_no_metadata"
   
       @register_global_func(func_name)
       def simple_func(x: int) -> int:
           return x + 1
   
       try:
           metadata = get_global_func_metadata(func_name)
           assert metadata == {}
       finally:
           remove_global_func(func_name)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to