help wanted
Repository metrics
- Stars
- (269 stars)
- PR merge metrics
- (PR metrics pending)
Description
The current integration testing code could benefit from some refactoring. The auto-numbering is a massive improvement, but there's still some minor annoyances:
- The output isn't intuitive, particularly for the fault-injection tests. Printing 'pass' or 'ok' after each successful test would be an improvement, but isn't really feasible without changing how output is handled (at the moment both the client and server share stdout).
- Fault injection tests indicate success with strings like 'Good Fault bad padding' which is obscure and could be interpreted as meaning the test failed, particularly since most tests don't produce any output at all.
- Several tests have different names on the client and server (e.g. test 0 is called 'anonymous handshake' on the client and 'Anonymous server handshake' on the server).
- Typically only one end of the connection crashes, and the other then falls over with an unrelated error ('Connection Refused' or 'TLSAbruptCloseError'). It's generally obvious which end failed, but it's still less than ideal.
- If testing is interrupted (e.g. with SIGINT/ctrl-C) or something causes the tlstest client to crash, the server can be left running which causes subsequent
make testruns to fail ('Address already in use'). The exact circumstances for triggering this are unclear, beyond getting (un)lucky when you abort the test.
One option that comes to mind is to rewrite the tests to use the async API, which would remove the need for separate client and server processes. Testing could then be managed with the unittest module, to be consistent with the unit tests. Seem reasonable?
Also, a couple of minor notes related to testing in general:
- Running unit tests on Python 2 requires the 'mock' module to be installed, this doesn't appear to be in the list of requirements in CONTRIBUTING.md. Likewise, epydoc (and the associated modifications) are also missing from the list.
- The current
make testtarget doesn't seem particularly useful given that it's undocumented and tests the installed copy of tlslite instead of the local copy. It also doesn't run unit tests and only runs the test on the default python. Perhaps the 'test' and 'test-local' targets should be removed, and 'test-dev' split up so that 'test' can invoke the unit/integration tests without running the linter/coverage checker.