Conversation
Signed-off-by: hang lv <xlv20@fudan.edu.cn>
JiwaniZakir
left a comment
There was a problem hiding this comment.
In tests/test_examples.py, subprocess.Popen is called without stdout=subprocess.PIPE, stderr=subprocess.PIPE, so when service.communicate() returns on line 78, both stdout and stderr will be None. The failure message assert code == 0, (code, stdout, stderr) will print (code, None, None), providing no useful diagnostic information when a test fails — consider adding those pipe arguments to the Popen call.
The test function is named test_forward_timeout, which is misleading since there's no timeout behavior being tested here; a name like test_example_inference would better reflect what's actually being validated.
In examples/client.py, the condition if k not in ["msgpack"] on line 43 is effectively dead code — "msgpack" is not a key in the req dict, so this branch will never be taken, and the content will always be JSON-encoded. Either the req dict is missing a "msgpack" entry, or this condition should be removed. Additionally, the error message on line 40 says "Please specify a shm to run: plasma or redis" but echo is also a valid key, which will mislead users.
There's also a minor style inconsistency: examples/type_validation/client.py uses the bare exit(1) builtin while the other clients use sys.exit(1) — sys.exit is the preferred form in library/script code.
ref to #181
Description
Add tests for all runnable examples.
Take the ret_code of client.py as the test result, so a little changes to the existing code are necessary.