ci: S13.5 bdd-windows job running walking-skeleton on windows-2025#156
Conversation
Extends windows-build-and-test to also build SolidSyslogWindowsExample and upload the resulting SolidSyslogExample.exe as an artefact. Adds a new bdd-windows job that: - Runs on windows-2025 (explicitly pinned; windows-latest still maps to 2022 for many accounts, and we want to pin intent). - Depends on windows-build-and-test, downloads the exe artefact. - Installs Python 3.13 via actions/setup-python, then behave via the pinned Bdd/requirements.txt. - Installs otelcol-contrib via Bdd/otel/Install-OtelCollector.ps1 (SHA-256 pinned inside the script). - Starts otelcol-contrib as a hidden Start-Process so it persists across steps and writes stdout/stderr to Bdd/output/ for failure diagnosis. - Runs behave with the tag filter `not @wip and not @windows_wip and not @buffered` — today that's just the walking-skeleton scenario; further @udp features join automatically as they're promoted out of @windows_wip. - Publishes JUnit XML via dorny/test-reporter and uploads it as junit-bdd-windows for the Quality Monitor summary. bdd-windows added to quality-summary needs list and a new "BDD Tests (Windows)" entry added to the Quality Monitor config; existing "BDD Tests" renamed to "BDD Tests (Linux)" for symmetry. Refs #129 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWindows CI now builds and uploads a Windows example executable and adds a Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant WinBuilder as "windows-build-and-test (Runner)"
participant Artifacts as "GitHub Artifact Store"
end
rect rgba(200,255,200,0.5)
participant BDDJob as "bdd-windows (Runner)"
participant PyEnv as "Python / Behave"
participant OTel as "OpenTelemetry Collector"
participant Reporter as "Test Reporter / JUnit upload"
end
WinBuilder->>WinBuilder: Build `SolidSyslogWindowsExample`
WinBuilder->>Artifacts: Upload `SolidSyslogExample.exe`
BDDJob->>Artifacts: Download `SolidSyslogExample.exe`
BDDJob->>PyEnv: Install Python 3.13 and BDD deps
BDDJob->>OTel: Install + start otelcol-contrib.exe
OTel-->>BDDJob: Bind UDP 5514 (ready)
BDDJob->>PyEnv: Run `behave` on Bdd/features/ (with tag exclusions)
PyEnv->>Reporter: Publish JUnit BDD results
BDDJob->>Artifacts: Upload `junit-bdd-windows` (and collector logs on failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 403-404: The CI step named "Wait for oracle to bind UDP 5514" uses
a fixed sleep which causes flakiness; replace that single "run: sleep 3" step
with a readiness probe that polls the target (e.g., test UDP port 5514 or the
OTel health endpoint) in a loop with a timeout and short sleeps between
attempts; update the step to exit successfully only when the probe detects the
port/service is ready and to fail after a configurable timeout so the job is
deterministic (refer to the "Wait for oracle to bind UDP 5514" step when
locating the change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
☀️ Quality Summary 🚦 Unit Tests (GCC): 99% successful (✔️ 539 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Addresses the bdd-windows failure in the first run of this PR (asserted "oracle received 0 of 1 messages within 5 seconds" with both otelcol.out and otelcol.err empty — clear sign the collector either didn't start or its output went somewhere we weren't reading). Root cause hypothesis: Start-Process on pwsh/windows-2025 doesn't reliably propagate \$PWD to the child's cwd or to -RedirectStandard* file resolution, so `--config=Bdd/otel/config.yaml` and the redirect paths landed unpredictably. Fix: resolve all paths to absolute upfront and pass -WorkingDirectory explicitly. Also replaces the 3-second sleep with an active UDP-5514 readiness probe via Get-NetUDPEndpoint. Exits fast when the collector binds, dumps otelcol.out/.err and fails with a clear message on timeout. Either outcome gives us a definitive signal on the next run: - green: path resolution was indeed the bug; walking-skeleton passes. - red: logs now show why otelcol isn't binding. Also addresses CodeRabbit's sleep-flakiness comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 Unit Tests (GCC): 99% successful (✔️ 539 passed, 🙈 3 skipped) 🚧 Error MessagesCreated by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Previous pwsh Start-Process attempt returned successfully on windows-2025 but the child either exited silently or wasn't launched — both otelcol.out and otelcol.err stayed empty even after 30s, and no process bound UDP 5514. Switch to bash `nohup ... &` which is exactly what works on the local Windows setup (Git Bash is installed on windows-2025 and is already the job's default shell). Print the backgrounded PID for traceability. Also enrich the timeout branch of the readiness probe: dump tasklist, the top 20 UDP endpoints, the Bdd/output directory listing, and both otelcol log files. If otelcol is running but not binding, tasklist will show it; if otelcol is dead, it won't. Either tells us which class of problem we're dealing with on the next red run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 Unit Tests (GCC): 99% successful (✔️ 539 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
The Windows example now accepts --facility/--severity/--msgid/--message via a tiny argv parser. Extracted to its own translation unit (Example/Windows/ExampleWindowsCommandLine.c) so it has a unit-testable surface — TDD discipline mirroring the POSIX ExampleCommandLine.c + ExampleCommandLineTest.cpp pattern. Inline-in-main was a discipline miss caught in review. Tests/CMakeLists.txt now adds Tests/Example/ when either POSIX or (WINSOCK + Windows platform) is configured. Tests/Example/CMakeLists.txt conditionally pulls in the right test files and production sources per platform: ExampleAppNameTest always, the existing four POSIX tests when SOLIDSYSLOG_POSIX, and the new ExampleWindowsCommandLineTest when SOLIDSYSLOG_WINSOCK and HAVE_WINDOWS_PLATFORM. The ExampleTests target now builds and runs on the windows-build-and-test job too. parse_otel_jsonl_line now reads OTel's `message` and `msg_id` attributes (confirmed via the parser experiment in #156's discussion: OTel's RFC 5424 receiver is an independent oracle in the same role syslog-ng plays on Linux). Verified locally: - ExampleWindowsCommandLine unit tests on MSVC: 11 / 11 green - All ExampleTests on Linux gcc: 41 / 41 green - BDD on Windows via otelcol oracle: 5 features / 14 scenarios / 52 steps - BDD on Linux via docker-compose: 14 features / 31 scenarios / 160 steps unchanged - tidy, cppcheck, sanitize, format: all green Refs #129 (follow-up promotion work) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Windows example now accepts --facility/--severity/--msgid/--message via a tiny argv parser. Extracted to its own translation unit (Example/Windows/ExampleWindowsCommandLine.c) so it has a unit-testable surface — TDD discipline mirroring the POSIX ExampleCommandLine.c + ExampleCommandLineTest.cpp pattern. Inline-in-main was a discipline miss caught in review. Tests/CMakeLists.txt now adds Tests/Example/ when either POSIX or (WINSOCK + Windows platform) is configured. Tests/Example/CMakeLists.txt conditionally pulls in the right test files and production sources per platform: ExampleAppNameTest always, the existing four POSIX tests when SOLIDSYSLOG_POSIX, and the new ExampleWindowsCommandLineTest when SOLIDSYSLOG_WINSOCK and HAVE_WINDOWS_PLATFORM. The ExampleTests target now builds and runs on the windows-build-and-test job too. parse_otel_jsonl_line now reads OTel's `message` and `msg_id` attributes (confirmed via the parser experiment in #156's discussion: OTel's RFC 5424 receiver is an independent oracle in the same role syslog-ng plays on Linux). Verified locally: - ExampleWindowsCommandLine unit tests on MSVC: 11 / 11 green - All ExampleTests on Linux gcc: 41 / 41 green - BDD on Windows via otelcol oracle: 5 features / 14 scenarios / 52 steps - BDD on Linux via docker-compose: 14 features / 31 scenarios / 160 steps unchanged - tidy, cppcheck, sanitize, format: all green Refs #129 (follow-up promotion work) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Packages the now-known-good local recipe from #154 into a CI job.
windows-build-and-testto build theSolidSyslogWindowsExampletarget and uploadSolidSyslogExample.exeas thesolid-syslog-windows-exampleartefact.bdd-windowsjob on pinnedwindows-2025that:build/msvc-debug/Example/Debug/,actions/setup-python@v6.2.0(pinned SHA),behave==1.3.3viaBdd/requirements.txt(same version as the Linux container image),otelcol-contrib v0.150.1viaBdd/otel/Install-OtelCollector.ps1(SHA-256 pinned),Start-Processso it persists across steps and writesstdout/stderrto files for failure diagnosis,behave --tags='not @wip and not @windows_wip and not @buffered'— today that's just the walking-skeleton scenario,dorny/test-reporterand uploadsjunit-bdd-windowsfor the Quality Monitor summary.quality-summarywired: addsbdd-windowstoneeds:and a newBDD Tests (Windows)entry to the quality-monitor config. ExistingBDD Testsentry renamed toBDD Tests (Linux)for symmetry.As more
@udpfeatures are promoted out of@windows_wipin subsequent PRs, they'll automatically start running on this Windows CI job — no yaml change needed.Why
windows-2025notwindows-latestwindows-lateststill maps towindows-2022for many accounts; pinning towindows-2025captures intent and avoids silent drift when GitHub eventually re-tags. The unit-test job (windows-build-and-test) stays onwindows-latest— it doesn't care, and the artefact is forward-compatible.Design notes
Pattern choice is straightforward serial (
needs: windows-build-and-test). I considered parallel rebuilds or splitting the build job to let tests and BDD run concurrently, but the wall-clock gain vs current CI times is ~5 s — build dominates either way. Can revisit if unit-test runtime grows past a minute or two.Default shell for the new job is
bash, matching the local Git-Bash recipe documented indocs/bdd.md(inlineVAR=value commandinvocation works natively). PowerShell is used only for PowerShell-native steps (the install script andStart-Process).Verified locally
actionlint(Docker): no new warnings introduced. Pre-existing shellcheck advisories on the unchangedformatandcppcheckjobs remain.Refs
Refs #129
Test plan
windows-build-and-teststill passes and now uploadssolid-syslog-windows-exampleartefactbdd-windowsstatus check appears and passesBDD Tests (Windows)row with 1 passed / 0 failed🤖 Generated with Claude Code
Summary by CodeRabbit