Pytest tests and CI - #58
Conversation
fbd0f44 to
86dea61
Compare
| conn.sendall(buf) | ||
| time.sleep(0.01) # sadly without this, microcom may miss the transmission | ||
| sock.close() |
There was a problem hiding this comment.
Could this hardcoded sleep result in flaky tests in the future?
There was a problem hiding this comment.
Yes, definitely. I'm not sure why this is even needed. I'd expect that expect blocks until microcom has connected via TCP and that from this point whatever the server sends should definitely reach microcom. I suspect this is actually a workaround for a microcom bug.
As long as it's needed, should 0.01 turn out flaky on a slow runner, 0.1 should still run the tests quick enough.
But yes bad bad hack!
|
I like it! Since this adds python code to the repository: should the CI also run e.g. |
| @pytest.fixture | ||
| def telnet_recv(cmd): | ||
| def _recv(buf, timeout=1): |
There was a problem hiding this comment.
Define helper functions directly; there is no need to use fixtures for that.
There was a problem hiding this comment.
The reason that this is a fixture and not a helper function is that it directly depends on the cmd fixture. My understanding is that only tests and fixtures can depend on other fixtures, therefore a fixture is adequate here.
As there's little reason why a test that uses telnet_recv would also use cmd, there is no reason to pass the cmd fixture to the test only to then pass it on to telnet_recv.
So I'd prefer this:
def test_a(telnet_recv):
assert telnet_recv("abc") == "abc"Over this:
def test_a(cmd):
assert telnet_recv(cmd, "abc") == "abc"The difference is negligible in this case but adding more dependent fixtures to telnet_recv in the latter solution would be suboptimal, because they'd all need to be added to each of the tests even if the test code itself isn't touched
There was a problem hiding this comment.
Using a fixture for helper functions such as these hides that telnet_recv actually uses the cmd fixture, making the code unnecessarily hard to read.
|
Thanks! |
The quiet switch silences all printing to stdout during normal operation, but leaves the interactive console and all error messages functional. Useful for automation such as in CI. Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
I set up those tests for the issues with the current telnet implementation. They can serve as regression tests after the issues have been resolved. Add them as the first integration tests. Add pythons bytecode cache files to gitignore. Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
- fix dependency installation - invoke pytest - don't build an unneeded release tarball Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
This has proven to be useful to catch out-of-bounds access and I hope can prevent some cases of success by chance when testing microcom. Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
Enforce the C style from uncrustify.cfg. Format one line that didn't follow the style yet. Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
Amongst others, this catches uninitialized memory access, which libasan does not. Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
3 participants
I've done some cleanup in the microcom codebase, and created a CI workflow to catch many of the encountered issues in the future.
These tests fail now, as long as the PRs resolving the issues are still open:
telnet.cto correctly handle command sequences at buffer boundaries, see telnet.c: rewrite command handling #57