Rewrite tests to python for windows runtime testing#215
Rewrite tests to python for windows runtime testing#215julek-wolfssl wants to merge 30 commits intowolfSSL:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the wolfCLU test suite from shell scripts to Python unittest modules to enable reliable runtime testing on Windows, while also updating Windows build configuration and CI workflows.
Changes:
- Replaced many
tests/**/*.shscripts with equivalenttests/**/*-test.pyPython unittest modules and added a Windows-friendlytests/run_tests.pyrunner. - Updated Automake integration to recognize
.pytests and added Python detection inconfigure.ac. - Updated Windows Visual Studio project/solution and CI workflows; added Windows networking initialization and SNI handling in the client setup.
Reviewed changes
Copilot reviewed 93 out of 94 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfclu/clu_header_main.h | Undefines CRL reason macros before wolfSSL headers (Windows build hygiene). |
| wolfCLU.vcxproj.filters | Adds new source files to the Visual Studio filters list. |
| wolfCLU.vcxproj | Updates toolset and enables multiprocessor compilation. |
| wolfclu.sln | Updates VS version metadata and fixes x64 config mapping. |
| tests/x509/x509-verify-test.sh | Removed (replaced by Python unittest). |
| tests/x509/x509-verify-test.py | New Python unittest for wolfssl verify. |
| tests/x509/x509-req-test.sh | Removed (replaced by Python unittest). |
| tests/x509/x509-process-test.sh | Removed (replaced by Python unittest). |
| tests/x509/x509-ca-test.sh | Removed (replaced by Python unittest). |
| tests/x509/include.am | Registers Python x509 tests in Automake. |
| tests/x509/CRL-verify-test.sh | Removed (replaced by Python unittest). |
| tests/x509/CRL-verify-test.py | New Python unittest for wolfssl crl. |
| tests/wolfclu_test.py | Shared helper to locate wolfssl binary and run commands. |
| tests/testEncDec/test_aesctr.sh | Removed (replaced by Python unittest). |
| tests/testEncDec/test_aescbc_3des_cam.sh | Removed (replaced by Python unittest). |
| tests/testEncDec/README.md | Removed (test approach changed to self-contained Python). |
| tests/testEncDec/include.am | Registers new enc/dec Python test in Automake. |
| tests/testEncDec/encdec-test.py | New encrypt/decrypt round-trip tests for multiple ciphers. |
| tests/server/server-test.sh | Removed (replaced by Python unittest). |
| tests/server/server-test.py | New Python unittest for s_server/s_client handshake. |
| tests/server/include.am | Registers Python server test in Automake. |
| tests/run_tests.py | New Windows-oriented Python test runner discovering *-test.py. |
| tests/rand/rand-test.sh | Removed (replaced by Python unittest). |
| tests/rand/rand-test.py | New Python unittest for wolfssl rand. |
| tests/rand/include.am | Registers Python rand test in Automake. |
| tests/pkey/rsa-test.sh | Removed (replaced by Python unittest). |
| tests/pkey/rsa-test.py | New Python unittest for RSA key conversion/validation. |
| tests/pkey/pkey-test.sh | Removed (replaced by Python unittest). |
| tests/pkey/pkey-test.py | New Python unittest for pkey public/private conversions. |
| tests/pkey/include.am | Registers Python pkey tests in Automake. |
| tests/pkey/ecparam-test.sh | Removed (replaced by Python unittest). |
| tests/pkey/ecparam-test.py | New Python unittest for curve enumeration and key generation. |
| tests/pkcs/pkcs8-test.sh | Removed (replaced by Python unittest). |
| tests/pkcs/pkcs8-test.py | New Python unittest for PKCS8 conversion and error cases. |
| tests/pkcs/pkcs7-test.sh | Removed (replaced by Python unittest). |
| tests/pkcs/pkcs7-test.py | New Python unittest for PKCS7 conversions and printing certs. |
| tests/pkcs/pkcs12-test.sh | Removed (replaced by Python unittest). |
| tests/pkcs/pkcs12-test.py | New Python unittest for PKCS12 extraction options. |
| tests/pkcs/include.am | Registers Python pkcs tests in Automake. |
| tests/ocsp/ocsp-test.sh | Removed (replaced by Python unittest). |
| tests/ocsp/ocsp-test.py | New combined OCSP interop test suite in Python. |
| tests/ocsp/include.am | Registers Python OCSP test in Automake. |
| tests/ocsp-scgi/scgi_params | Removed (nginx dependency eliminated). |
| tests/ocsp-scgi/ocsp-scgi-test.py | New Python HTTP-to-SCGI proxy test replacing nginx. |
| tests/ocsp-scgi/include.am | Registers Python OCSP-SCGI test in Automake. |
| tests/hash/include.am | Switches hash tests from .sh to .py. |
| tests/hash/hash-test.sh | Removed (replaced by Python unittest). |
| tests/hash/hash-test.py | New Python unittest for -hash and shortcut hash commands. |
| tests/genkey_sign_ver/include.am | Switches genkey/sign/verify test registration to Python. |
| tests/genkey_sign_ver/genkey-sign-ver-test.sh | Removed (replaced by Python unittest). |
| tests/genkey_sign_ver/genkey-sign-ver-test.py | New Python unittest for keygen + sign/verify workflows incl. PQC. |
| tests/encrypt/include.am | Switches encrypt tests from .sh to .py. |
| tests/encrypt/enc-test.sh | Removed (replaced by Python unittest). |
| tests/encrypt/enc-test.py | New Python unittest for enc/dec, interop, and legacy names. |
| tests/dsa/include.am | Switches DSA tests from .sh to .py. |
| tests/dsa/dsa-test.sh | Removed (replaced by Python unittest). |
| tests/dsa/dsa-test.py | New Python unittest for dsaparam behaviors. |
| tests/dh/include.am | Switches DH tests from .sh to .py. |
| tests/dh/dh-test.sh | Removed (replaced by Python unittest). |
| tests/dh/dh-test.py | New Python unittest for dhparam behaviors. |
| tests/dgst/include.am | Switches dgst tests from .sh to .py. |
| tests/dgst/dgst-test.sh | Removed (replaced by Python unittest). |
| tests/dgst/dgst-test.py | New Python unittest for signature verification + large-file cases. |
| tests/client/include.am | Switches client test from .sh to .py. |
| tests/client/client-test.sh | Removed (replaced by Python unittest). |
| tests/client/client-test.py | New Python unittest for external TLS connection + x509 extraction. |
| tests/bench/include.am | Switches bench test from .sh to .py. |
| tests/bench/bench-test.sh | Removed (replaced by Python unittest). |
| tests/bench/bench-test.py | New Python unittest for -bench smoke tests. |
| tests/base64/include.am | Switches base64 test from .sh to .py. |
| tests/base64/base64-test.sh | Removed (replaced by Python unittest). |
| tests/base64/base64-test.py | New Python unittest for base64 encode/decode + stdin coverage. |
| src/x509/clu_x509_sign.c | Adjusts unsupported-hash handling in cert signing code path. |
| src/tools/clu_funcs.c | Gates AES-CTR help output on wolfSSL version macro. |
| src/server/clu_server_setup.c | Calls StartTCP() before running server test (Windows networking). |
| src/ocsp/clu_ocsp.c | Calls StartTCP() before OCSP client/responder execution. |
| src/crypto/clu_decrypt.c | Adjusts file-read loop behavior for decrypt path. |
| src/client/clu_client_setup.c | Adds SNI args and calls StartTCP() before running client test. |
| src/client/client.c | Implements Windows checkStdin() logic and enables it on Windows. |
| Makefile.am | Adds .py test extensions and includes new testEncDec include.am. |
| ide/winvs/user_settings.h | Enables SNI/TLS13/PKCS/CRL/OCSP features for Windows builds. |
| configure.ac | Adds Python discovery/substitution for Automake test harness. |
| .gitignore | Ignores Visual Studio build outputs and Python cache dirs. |
| .github/workflows/windows-check.yml | Modernizes Windows build + runs new Python test runner. |
| .github/workflows/ubuntu-check.yml | Removed (replaced by consolidated CI workflow). |
| .github/workflows/macos-check.yml | Removed (replaced by consolidated CI workflow). |
| .github/workflows/fsanitize-check.yml | Removed (replaced by consolidated CI workflow matrix). |
| .github/workflows/ci.yml | New consolidated Ubuntu/macOS matrix build + make check. |
| .gitattributes | Enforces LF for PEM/config files; marks DER as binary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 93 out of 94 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
774573b to
e7e1d5d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 95 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/crypto/clu_decrypt.c:166
- The read loop now treats only
XFREAD(...) > 0as success and logs "Input file does not exist" for a 0-byte read. Becausefeof(inFile)is checked before attempting the read, EOF will typically be detected only afterXFREADreturns 0; with this change that becomes a hard error instead of being handled as EOF. Consider checkingfeof()/ferror()after the read (or handlingret == 0 && feof(inFile)separately) so valid EOF conditions don’t get reported as a missing file.
}
else {
ret = (int)XFREAD(input, 1, MAX_LEN, inFile);
if (ret > 0) {
tempMax = ret;
ret = 0; /* success */
}
else {
wolfCLU_LogError("Input file does not exist.");
ret = FREAD_ERROR;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1e6209e to
85be9ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 98 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
wolfclu.sln:1
- The solution file now starts with an empty first line, and
VisualStudioVersionincludes a non-numeric suffix (stable)..slnparsing is picky: the first line is expected to be the header, andVisualStudioVersionis typically a numeric version string. Please remove the leading blank line and ensureVisualStudioVersionis in the standard numeric format to avoid VS/MSBuild failing to load the solution.
tests/ocsp-scgi/ocsp-scgi-test.py:1 - The SCGI/HTTP proxy tests use fixed ports (6961/8089). This can cause intermittent CI failures if those ports are already in use on the runner (or if the suite is run concurrently). Prefer binding to ephemeral ports (bind to port 0, then read back the assigned port) and pass the chosen ports through to both the proxy and the wolfssl responder invocation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2f7b724 to
5ac2054
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 98 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/crypto/clu_decrypt.c:172
- When
XFREAD()returns 0, the code reports "Input file does not exist.". At this point the file was already opened successfully, so this message is misleading and will obscure real causes (unexpected EOF vs read error). Update this branch to emit an accurate read/EOF error and handleret == 0(EOF) distinctly fromret < 0/error.
ret = (int)XFREAD(input, 1, MAX_LEN, inFile);
if (ret > 0) {
tempMax = ret;
ret = 0; /* success */
}
else {
wolfCLU_LogError("Input file does not exist.");
ret = FREAD_ERROR;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Windows failure is unrelated to this PR. Fixed in wolfSSL/wolfssl#10161. |
Replace fsanitize-check.yml, macos-check.yml, and ubuntu-check.yml with a single ci.yml that uses a matrix of OS, config, and sanitizer dimensions. Update all checkout actions to v4 and setup-msbuild to v2.
Simplify windows-check.yml: remove ${{env.GITHUB_WORKSPACE}} indirection,
drop PlatformToolset/WindowsTargetPlatformVersion overrides, add timeout,
pin wolfSSL ref, consolidate env vars. Update VS project to v145 toolset,
add missing source files to filters, enable multi-processor compilation.
- Use $(DefaultPlatformToolset) in vcxproj instead of hard-coded v145 - Remove toolset detection step from Windows CI (no longer needed) - Disable Python tests when Python 3 not found instead of failing build - Add -noservername flag to s_client (SNI on by default, matching openssl) - Fix wolfclu_test.py paths to be __file__-relative for location independence - Add trailing newline to pkcs12 stdin password inputs - Redirect s_server stdout/stderr to DEVNULL to prevent pipe deadlock
Add test_main() helper in wolfclu_test.py that runs unittest with exit=False and translates the result: all-skipped/no-tests -> exit 77 (automake SKIP), failures -> exit 1, otherwise -> exit 0.
wc_CamelliaSetKey was passed `block` (cipher block size, always 16) instead of `size / 8` (key size in bytes). The `size` parameter is in bits (128/192/256) so it must be divided by 8. The old code happened to work for camellia-128 since block == 128/8 == 16.
- Fix decrypt lastLoopFlag to use payload size excluding salt+IV, preventing spurious EOF error when payload < MAX_LEN - Handle WAIT_FAILED/WAIT_ABANDONED in Windows checkStdin() - Catch TimeoutExpired in server-test.py cleanup, fall back to kill() - Also exit 77 when all individual tests are skipped (not just class) - Increase MAX_CLIENT_ARGS from 17 to 32 for worst-case arg expansion - Remove trailing whitespace on #undef lines in clu_header_main.h
- Move SNI arg insertion to after option parsing so -noservername works regardless of argument order relative to -connect - Validate input file is large enough for salt+IV before computing lastLoopFlag to prevent negative values on truncated files
s_client may return non-zero when peer verification fails (no CA bundle loaded), but the connection still succeeds and the server certificate is printed to stdout. Assert on the certificate being present rather than requiring exit code 0.
- Use absolute paths with forward slashes for all temp files via _tmp() helper so wolfSSL recognizes them correctly - Use explicit UTF-8 encoding with LF newlines when writing config files so wolfSSL's NCONF_load can parse them on Windows - Remove $dir variable expansion in config templates; use direct absolute paths to CERTS_DIR instead - Pass bare filenames to ca -out (not absolute paths) since wolfSSL prepends new_certs_dir and doesn't recognize drive-letter paths as absolute - Export PROJECT_ROOT from wolfclu_test.py for test file path helpers
- Fix AES-CTR version check from > to >= 0x05009000 - Add HAVE_CAMELLIA to Windows user_settings.h - Override XINET_PTON to use InetPtonA (narrow-string) instead of wolfSSL's default which incorrectly casts char* to PCWSTR - Remove IPv6 SAN test skips now that InetPtonA works correctly
Adds WOLFSSL_SHA3, WOLFSSL_SHAKE128, WOLFSSL_SHAKE256, HAVE_DILITHIUM, and WOLFSSL_WC_DILITHIUM to user_settings.h. This enables ML-DSA (dilithium) keygen/sign/verify tests on Windows using wolfSSL's native implementation (no liboqs needed).
- Check for drive-letter (X:) and backslash (\) paths as absolute in ca -out handling, matching OpenSSL's ossl_is_absolute_path() logic with #ifdef _WIN32 guard - Add Windows-specific tests for drive-letter and backslash absolute paths in TestCAOutdirPath - Enable WOLFSSL_DUAL_ALG_CERTS for chimera/altextend CA tests - Fix MSVC VLA error: use #define for LARGE_TEMP_SZ instead of const int (MSVC requires compile-time constant for array sizes) - Replace CSR attributes test: use new attributes-supported-csr.pem with only wolfSSL-supported attributes (challengePassword, unstructuredName), add test asserting unsupported attributes fail
Use load_tests protocol to filter out _OCSPInteropBase which has CLIENT_BIN=None. Previously it showed as a confusing "binary not configured" skip.
- Default to P-256 when no curve name is given, so the EC_KEY group NID is always set before EC_KEY_generate_key(). Without this, wolfSSL_EC_KEY_new() leaves the group as WC_NID_undef and the generated key's DER export is malformed. - Increase test sign data from 15 to 20 bytes to meet wolfSSL's WC_MIN_DIGEST_SIZE (16) requirement in wc_ecc_sign_hash().
- Close outFile on early return in clu_decrypt.c to fix handle leak - Fix misleading "Input file does not exist" error message in decrypt - Add #undef LARGE_TEMP_SZ to prevent macro namespace pollution - Require path separator after drive letter in Windows absolute path check - Use dynamic port allocation in OCSP and OCSP-SCGI tests to avoid port conflicts
- Include <ctype.h> for isalpha() in Windows absolute path check - Reject zero-length payload (length <= saltAndIvSize) in decrypt - Assert x509 extraction succeeds and produces output file in client test
Ports the regression tests added in ec1c1e5 (Merge PR wolfSSL#212, F 569: Fix stack buffer overflow in encryption setup) from enc-test.sh to the python enc-test.py. Covers both the AES/EVP inName path and the Camellia non-EVP outNameEnc/outNameDec paths for: - Filename supplied via stdin when -in/-out is omitted - Empty-line reprompt after invalid input - Long-input flush (255 chars) after overflow attempt
The drive-letter branch indexed out[1] and out[2] relying on short-circuit evaluation of NUL-termination. Add an explicit outSz >= 3 length check before the indexing to avoid any potential out-of-bounds read that ASAN/UBSAN could flag.
32079c4 to
14cae20
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 98 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JacobBarthelmeh
left a comment
There was a problem hiding this comment.
Reviewed the changes and they look good, thanks! Please investigate make distcheck though. I'm seeing a failure when trying locally with these changes.
Python tests assumed the build and source directories were identical, which holds for an in-tree build but not for `make distcheck`, where automake builds in a separate _build/sub directory. Under distcheck the wolfssl binary was searched for under the source tree (where it did not exist), fixture files were opened via cwd-relative paths, and the certs/ tree was not distributed at all. As a result every Python test either failed to find the wolfssl binary or skipped because certs/ was missing. - Add certs/ to EXTRA_DIST so it ships in the distribution tarball. - Teach wolfclu_test.py to locate the wolfssl binary and certs/ via the build/source directories exported through AM_TESTS_ENVIRONMENT, with sensible fallbacks for direct script invocation. - Replace cwd-relative "./tests/<dir>" paths in dgst, hash, and x509-process tests with paths derived from each test file's location, so they resolve correctly regardless of where the test is run from.
No description provided.