-
Notifications
You must be signed in to change notification settings - Fork 247
Add unit testing harness #3828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add unit testing harness #3828
Changes from 16 commits
cbd9dc1
7ce4137
58bd238
1521225
94cbe34
8e2853b
ccbb7f5
0b5c9dc
6094fbe
e859ee9
9a26abc
eb2145b
383c143
ebb39ce
0df93f2
3b4612a
bc22665
33a8972
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,213 @@ | ||
| #!/usr/bin/env python3 | ||
| """Cross-platform build+run driver for the protocol unit test suite. | ||
|
|
||
| Usage: | ||
| python3 run-unit-tests.py build | ||
| qmake + make/nmake, out-of-tree in build-test/. On Windows this also | ||
| bootstraps the MSVC environment first (see apply_msvc_environment()): | ||
| build and run are separate GitHub Actions steps (each a fresh shell), so | ||
| that bootstrap has to happen again here rather than once in the workflow. | ||
|
|
||
| python3 run-unit-tests.py run | ||
| Runs the built binary, writes test-output.txt and test-results.xml, | ||
| prints the text report, then gates on test-results.xml: exits nonzero | ||
| unless it reports at least one test and zero failures/errors. The | ||
| binary's own exit code is ignored (see cmd_run()). | ||
|
|
||
| Reads from the environment (set by the workflow, from the job's matrix): | ||
| QMAKE_BIN -- qmake executable name (default "qmake") | ||
| QMAKE_EXTRA_ARGS -- extra qmake command line arguments, shell-quoted as one | ||
| string (e.g. QMAKE_CXXFLAGS+="--coverage"); split with | ||
| shlex before passing to qmake | ||
| """ | ||
|
|
||
| import os | ||
| import platform | ||
| import re | ||
| import shlex | ||
| import subprocess | ||
| import sys | ||
| import xml.etree.ElementTree as ET | ||
|
|
||
| BUILD_DIR = "build-test" | ||
| RESULTS_PATH = "test-results.xml" | ||
| OUTPUT_PATH = "test-output.txt" | ||
|
|
||
|
|
||
| def qmake_extra_args(): | ||
| return shlex.split(os.environ.get("QMAKE_EXTRA_ARGS", "")) | ||
|
|
||
|
|
||
| def run(cmd, **kwargs): | ||
| print("+ " + " ".join(cmd)) | ||
| subprocess.run(cmd, check=True, **kwargs) | ||
|
|
||
|
dtinth marked this conversation as resolved.
|
||
|
|
||
| def msvc_environment(): | ||
| """Returns the environment variables vcvarsall.bat x64 adds/changes, by | ||
| diffing `cmd /c set` before and after calling it -- the same env-diffing | ||
| technique windows/deploy_windows.ps1's Initialize-Build-Environment uses | ||
| for the same purpose.""" | ||
| vswhere = r"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" | ||
| vs_path = subprocess.run( | ||
| [ | ||
| vswhere, | ||
| "-latest", | ||
| "-prerelease", | ||
| "-products", | ||
| "*", | ||
| "-requires", | ||
| "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", | ||
| "-property", | ||
| "installationPath", | ||
| ], | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ).stdout.strip() | ||
| vcvarsall = os.path.join(vs_path, "VC", "Auxiliary", "Build", "vcvarsall.bat") | ||
|
|
||
| # subprocess.run ( ..., shell=True ) on Windows already runs the string | ||
| # through "%COMSPEC% /c <string>" itself, so cmd is passed as raw command | ||
| # text here, not prefixed with a literal "cmd /c" -- doing that doubles up | ||
| # the cmd.exe nesting and silently swallows vcvarsall.bat's effect on the | ||
| # "after" snapshot (before == after, so no variables get applied). | ||
| def snapshot(cmd): | ||
| result = subprocess.run(cmd, shell=True, capture_output=True, text=True) | ||
| if result.returncode != 0: | ||
| sys.exit( | ||
| "command failed (exit {}): {}\nstdout:\n{}\nstderr:\n{}".format( | ||
| result.returncode, cmd, result.stdout, result.stderr | ||
| ) | ||
| ) | ||
|
|
||
| env = {} | ||
| for line in result.stdout.splitlines(): | ||
| m = re.match(r"^([^=]+)=(.*)$", line) | ||
| if m: | ||
| env[m.group(1)] = m.group(2) | ||
| return env | ||
|
|
||
| before = snapshot("set") | ||
| after = snapshot('call "{}" x64 >nul && set'.format(vcvarsall)) | ||
|
|
||
| return {name: value for name, value in after.items() if before.get(name) != value} | ||
|
|
||
|
|
||
| def apply_msvc_environment(): | ||
| for name, value in msvc_environment().items(): | ||
| os.environ[name] = value | ||
|
|
||
| print("VCToolsInstallDir={}".format(os.environ.get("VCToolsInstallDir", ""))) | ||
|
|
||
|
|
||
| def cmd_build(): | ||
| os.makedirs(BUILD_DIR, exist_ok=True) | ||
| qmake_bin = os.environ.get("QMAKE_BIN", "qmake") | ||
|
|
||
| if platform.system() == "Windows": | ||
| apply_msvc_environment() | ||
| run( | ||
| [ | ||
| qmake_bin, | ||
| "..\\src\\test\\test.pro", | ||
| "CONFIG-=debug_and_release", | ||
| "CONFIG+=release", | ||
| "DESTDIR=.", | ||
| ] | ||
| + qmake_extra_args(), | ||
| cwd=BUILD_DIR, | ||
| ) | ||
| run(["nmake"], cwd=BUILD_DIR) | ||
| else: | ||
| run([qmake_bin, "../src/test/test.pro"] + qmake_extra_args(), cwd=BUILD_DIR) | ||
| run(["make", "-j{}".format(os.cpu_count() or 1)], cwd=BUILD_DIR) | ||
|
|
||
|
|
||
| def test_binary_path(): | ||
| name = "jamulus-test.exe" if platform.system() == "Windows" else "jamulus-test" | ||
| return os.path.join(BUILD_DIR, name) | ||
|
|
||
|
|
||
| def pick_junit_format(binary): | ||
| # QtTest's junit-flavoured logger is called "xunitxml" on Qt 5.15 and was | ||
| # renamed "junitxml" on Qt 6.x -- ask the binary itself via -help instead | ||
| # of hardcoding it by Qt version, so this keeps working if the name | ||
| # changes again. | ||
| help_text = subprocess.run([binary, "-help"], capture_output=True, text=True).stdout | ||
| return "junitxml" if "junitxml" in help_text else "xunitxml" | ||
|
|
||
|
|
||
| def read_suites(path): | ||
| root = ET.parse(path).getroot() | ||
| return [root] if root.tag == "testsuite" else list(root.findall("testsuite")) | ||
|
|
||
|
|
||
| def gate_on_results(): | ||
| if not os.path.exists(RESULTS_PATH): | ||
| return "gate failed: no {} was produced".format(RESULTS_PATH) | ||
|
|
||
| try: | ||
| suites = read_suites(RESULTS_PATH) | ||
| except ET.ParseError as e: | ||
| return "gate failed: {} could not be parsed ({})".format(RESULTS_PATH, e) | ||
|
|
||
| total_tests = sum(int(suite.get("tests", 0)) for suite in suites) | ||
| total_failed = sum( | ||
| int(suite.get("failures", 0)) + int(suite.get("errors", 0)) for suite in suites | ||
| ) | ||
|
|
||
| if total_tests == 0: | ||
| return "gate failed: {} reported 0 tests".format(RESULTS_PATH) | ||
|
|
||
| if total_failed != 0: | ||
| return "gate failed: {} reported {} of {} tests failed".format( | ||
| RESULTS_PATH, total_failed, total_tests | ||
| ) | ||
|
|
||
| print("gate passed: {} tests, 0 failures".format(total_tests)) | ||
| return None | ||
|
|
||
|
|
||
| def cmd_run(): | ||
| binary = test_binary_path() | ||
| junit_format = pick_junit_format(binary) | ||
| print("Using QtTest JUnit logger format: " + junit_format) | ||
|
|
||
| # remove stale reports so the gate below can only ever see this run's | ||
| # output, even if the binary crashes before writing anything | ||
| for path in [RESULTS_PATH, OUTPUT_PATH]: | ||
| if os.path.exists(path): | ||
| os.remove(path) | ||
|
|
||
| # "-o file,txt" not "-o -,txt": on windows-latest runners this binary's | ||
| # stdout comes back 0 bytes regardless of capture method (suspected: Qt's | ||
| # console detection on the inherited handle); QtTest's own file writer | ||
| # works there. Using it on Unix too avoids a platform branch. | ||
| subprocess.run( | ||
| [binary, "-o", OUTPUT_PATH + ",txt", "-o", RESULTS_PATH + "," + junit_format] | ||
| ) | ||
|
|
||
|
dtinth marked this conversation as resolved.
|
||
| if os.path.exists(OUTPUT_PATH): | ||
| with open(OUTPUT_PATH, encoding="utf-8", errors="replace") as f: | ||
| sys.stdout.write(f.read()) | ||
|
|
||
| # The binary's own exit code is unreliable on Windows runners, so it's | ||
| # ignored on every platform; test-results.xml is the sole source of truth. | ||
| error = gate_on_results() | ||
| if error: | ||
| sys.exit(error) | ||
|
|
||
|
|
||
| def main(): | ||
| if len(sys.argv) != 2 or sys.argv[1] not in ("build", "run"): | ||
| sys.exit("usage: run-unit-tests.py <build|run>") | ||
|
|
||
| if sys.argv[1] == "build": | ||
| cmd_build() | ||
| else: | ||
| cmd_run() | ||
|
Comment on lines
+172
to
+209
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Fail when the QtTest process exits nonzero. 🧰 Tools🪛 ast-grep (0.45.2)[warning] 191-191: File path is request-/variable-derived; validate and normalize to prevent path traversal. (open-filename-from-request) [error] 186-188: Command coming from incoming request (subprocess-from-request) 🪛 Ruff (0.16.3)[error] 187-187: (S603) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| #!/usr/bin/env python3 | ||
| """Renders test-results.xml (the junit-flavoured QtTest report | ||
| run-unit-tests.py's "run" subcommand writes) as a per-suite pass/fail table, | ||
| plus any failing test names/messages, appended to $GITHUB_STEP_SUMMARY. | ||
| Reporting only: always exits 0, so a missing or unparsable results file just | ||
| shows up in the summary instead of failing this step. | ||
| """ | ||
|
|
||
| import os | ||
| import sys | ||
| import xml.etree.ElementTree as ET | ||
|
|
||
| RESULTS_PATH = "test-results.xml" | ||
|
|
||
|
|
||
| def read_suites(path): | ||
| root = ET.parse(path).getroot() | ||
| return [root] if root.tag == "testsuite" else list(root.findall("testsuite")) | ||
|
|
||
|
|
||
| def write_summary(summary_path, lines): | ||
| if not summary_path: | ||
| sys.stdout.writelines(lines) | ||
| return | ||
|
|
||
| with open(summary_path, "a", encoding="utf-8") as f: | ||
| f.writelines(lines) | ||
|
|
||
|
|
||
| def main(): | ||
| job_name = os.environ.get("JOB_NAME", "unit tests") | ||
| summary_path = os.environ.get("GITHUB_STEP_SUMMARY") | ||
|
|
||
| lines = ["### JUnit results: {}\n\n".format(job_name)] | ||
|
|
||
| suites = None | ||
| if not os.path.exists(RESULTS_PATH): | ||
| lines.append( | ||
| "_No {} was produced (the build or test run likely failed before it could be written)._\n\n".format( | ||
| RESULTS_PATH | ||
| ) | ||
| ) | ||
| else: | ||
| try: | ||
| suites = read_suites(RESULTS_PATH) | ||
| except ET.ParseError as e: | ||
| lines.append("_{} could not be parsed ({})._\n\n".format(RESULTS_PATH, e)) | ||
|
|
||
| if suites is None: | ||
| write_summary(summary_path, lines) | ||
| return | ||
|
|
||
| lines.append("| Suite | Tests | Passed | Failed | Skipped | Time (s) |\n") | ||
| lines.append("|---|---|---|---|---|---|\n") | ||
|
|
||
| failures = [] | ||
|
|
||
| for suite in suites: | ||
| tests = int(suite.get("tests", 0)) | ||
| failed = int(suite.get("failures", 0)) + int(suite.get("errors", 0)) | ||
| skipped = int(suite.get("skipped", 0)) | ||
| passed = tests - failed - skipped | ||
| status = "PASS" if failed == 0 else "FAIL" | ||
|
|
||
| lines.append( | ||
| "| {} {} | {} | {} | {} | {} | {} |\n".format( | ||
| status, | ||
| suite.get("name"), | ||
| tests, | ||
| passed, | ||
| failed, | ||
| skipped, | ||
| suite.get("time", "?"), | ||
| ) | ||
| ) | ||
|
|
||
| for testcase in suite.findall("testcase"): | ||
| node = testcase.find("failure") | ||
| if node is None: | ||
| node = testcase.find("error") | ||
| if node is not None: | ||
| failures.append( | ||
| (testcase.get("name"), (node.get("message") or "").strip()) | ||
| ) | ||
|
|
||
| if failures: | ||
| lines.append("\n<details><summary>Failed tests</summary>\n\n") | ||
| for name, message in failures: | ||
| lines.append("- `{}`: {}\n".format(name, message)) | ||
| lines.append("\n</details>\n") | ||
|
|
||
| lines.append("\n") | ||
|
|
||
| write_summary(summary_path, lines) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -82,6 +82,11 @@ jobs: | |||||
| grep -oP '.*\K(?:ASIO-SDK|asiosdk)_.*(?=\.zip)' | ||||||
| local_version_regex: (.*["\/])((?:ASIO-SDK|asiosdk)_[^"]+?)(".*|\.zip.*) | ||||||
|
|
||||||
| - name: gcovr | ||||||
| # not Changelog-worthy | ||||||
| get_upstream_version: GH_REPO=gcovr/gcovr gh release view --json tagName --jq .tagName | sed -re 's/^v//' | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow context ---'
sed -n '70,135p' .github/workflows/bump-dependencies.yml
printf '%s\n' '--- relevant workflow references ---'
rg -n -C 3 'bump-dependencies|upstream_version|get_upstream_version|DEPLOY|deploy.?key|webfactory|ssh-key|GCOVR_VERSION' .github/workflows .github 2>/dev/null | head -240Repository: jamulussoftware/jamulus Length of output: 27109 🤖 get_repo_knowledge executed:
Length of output: 2658 Injection (CWE-94): Improper Control of Generation of Code ('Code Injection') Reachability: External · Exploitability: Difficult Validate the upstream version before Perl interpolation. A malicious Proposed fix- get_upstream_version: GH_REPO=gcovr/gcovr gh release view --json tagName --jq .tagName | sed -re 's/^v//'
+ get_upstream_version: GH_REPO=gcovr/gcovr gh release view --json tagName --jq .tagName | sed -re 's/^v//' | grep -Ex '[0-9]+(\.[0-9]+)*'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| local_version_regex: (.*GCOVR_VERSION:\s*)([0-9.]+)(.*) | ||||||
|
|
||||||
| steps: | ||||||
| - uses: actions/checkout@v7 | ||||||
| with: | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge jamulussoftware/jamulus /tmp/coderabbit-repo-knowledge/jamulussoftware-jamulus-e32ae91c/conventionsLength of output: 3190
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 14701
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 14287
🤖 get_repo_knowledge executed:
get_repo_knowledge jamulussoftware/jamulus /tmp/coderabbit-repo-knowledge/jamulussoftware-jamulus-e32ae91c/conventionsLength of output: 7487
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 29643
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 293
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 11288
🏁 Script executed:
Repository: jamulussoftware/jamulus
Length of output: 1555
Anchor and unify the build-version grammar.
.github/autobuild/mac.sh:67is unanchored, and.github/autobuild/windows.ps1:91permits arbitrary suffixes. Both accept1.2.3/other. The artifact functions then interpolate that value intodeploy/..., which can create an invalid nested destination and make artifact transfer fail.Use the same anchored expression in both validators. Allow
N.N.Nor theN.N.Ndev-<git-describe output>value produced by.github/autobuild/get_build_vars.py. Reject path separators and all other suffixes.📍 Affects 2 files
.github/autobuild/mac.sh#L67-L67(this comment).github/autobuild/windows.ps1#L91-L91🤖 Prompt for AI Agents