Add Smoke Tests as part of PR verification#47
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add test/smoke-tests.ps1: PowerShell harness that enumerates all JSON specs under openapi/v2.0, v3.0, v3.1, v3.2 and runs each through all four resource-wrapper modes (none, tags, paths, hybrid) = 88 cases total - Resolves repo root from \ so it is CWD-independent - Builds binary once with -Doptimize=ReleaseFast before running cases - Writes outputs to test/output/<version>/<basename>__<mode>.zig - Continue-all failure policy: collects all failures, prints summary, exits non-zero only for non-denylisted failures - Initial denylist: openapi/v3.0/ingram-micro.json (all modes) due to duplicate pub const emissions in the unified model generator - Add test/output/ to .gitignore to exclude generated artifacts from VCS Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Append two steps to the existing smoke-tests job in ci.yml:
1. 'Run broad smoke-test script': runs ./smoke-tests.ps1 from the test/
working directory using shell: pwsh (PowerShell Core, cross-platform)
2. 'Upload smoke-test outputs on failure': uploads test/output/ as artifact
'smoke-test-output' with retention-days: 7, guarded by if: failure()
- Petstore harness (zig build run-generate + zig run generated/main.zig) is
preserved unchanged; smoke tests run after it in the same job
- No trigger, runner, cache, or strategy changes
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a 'Smoke tests' subsection under Development in README.md covering: - How to run: pwsh test/smoke-tests.ps1 - Scope: all JSON specs under openapi/v2.0, v3.0, v3.1, v3.2 - Resource-wrapper matrix (none, tags, paths, hybrid) - Skips: YAML files and openapi/json-schema/ meta-schemas - Output layout under test/output/ (gitignored) - Continue-all failure policy and summary output - Denylist behavior for known-unsupported spec/mode combinations - CI behavior: runs in smoke-tests job on PRs and main; uploads test/output/ as workflow artifact when the job fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughA PowerShell smoke-test harness is introduced to validate code generation across all eligible OpenAPI specifications with multiple resource-wrapper modes. The script integrates into CI, discovered specs are tested end-to-end (generation through compilation), failures are denylisted and logged, and test outputs are captured as artifacts on failure. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Script as smoke-tests.ps1
participant CLI as openapi2zig CLI
participant Zig as zig compiler
participant CI as CI System
participant Artifacts as Artifact Store
User->>Script: Run (with -Filter, -Modes)
Script->>Script: Build openapi2zig (ReleaseFast)
Script->>Script: Discover JSON specs in openapi/*
Script->>Script: Filter specs & check denylist
loop Each (Spec, Mode) pair
alt Denylisted
Script->>Script: Log SKIP with reason
else Not denylisted
Script->>CLI: openapi2zig generate --resource-wrappers <mode>
CLI-->>Script: Generate .zig file or error
alt Generation successful
Script->>Zig: zig test <generated-file>
Zig-->>Script: Compilation result (pass/fail)
Script->>Script: Record pass or fail
else Generation failed
Script->>Script: Record failure
end
end
end
Script->>Script: Aggregate results & print summary
alt Any failures occurred
Script->>CI: Exit code 1
CI->>Artifacts: Upload test/output/
Artifacts-->>User: Artifacts available
else All passed or skipped
Script->>CI: Exit code 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.squad/agents/fenster/history.md:
- Around line 111-112: The implementation notes contain corrupted command/path
text; open .squad/agents/fenster/history.md and fix the mangled lines
referencing test/smoke-tests.ps1, $PSScriptRoot, Set-Location, Get-Command zig,
the zig build command (-Doptimize=ReleaseFast) and the executable selection
token (openapi2zig.exe vs openapi2zig or the flag used to select it), restoring
the original verbatim shell/PowerShell snippets and paths so commands are
accurate and consistent (e.g., "test/smoke-tests.ps1 resolves repo root via
$PSScriptRoot/.. and Set-Location", "Get-Command zig", "zig build
-Doptimize=ReleaseFast", and the correct token used to choose openapi2zig.exe).
Ensure encoding/escaping is correct so backslashes and flags render literally.
In @.squad/agents/juno/history.md:
- Around line 76-80: Update the history entry text under the "Scope" and "Key
findings" sections so the spec counts are consistent: either change the initial
"24 specs" and the breakdown "8+9+2+2" to match "22 specs shipped", or update
"22 specs shipped" to "24 specs shipped" and keep the 8+9+2+2 breakdown; ensure
both the summary line and the later sentence (the occurrence referenced at lines
127-129) use the same total and breakdown values so the documented coverage is
unambiguous.
In @.squad/agents/lando/history.md:
- Line 97: Open the .squad/agents/lando/history.md file and remove the stray
control characters that are corrupting text; specifically restore the missing
leading "t" in occurrences of "test/smoke-tests.ps1" (replace any
"est/smoke-tests.ps1" instances) and correct the corrupted token
"refAllDeclsRecursive" where it appears; ensure you search for and replace any
invisible control characters around those tokens so the file contains the exact
strings "test/smoke-tests.ps1" and "refAllDeclsRecursive" (check the lines
previously noted and any other occurrences) and save.
In @.squad/agents/scribe/history.md:
- Line 27: Update the malformed script path string "est/smoke-tests.ps1" in
.squad/agents/scribe/history.md to the correct path "test/smoke-tests.ps1" so
the shipped note accurately reflects the script name; locate the line containing
the incorrect token (the phrase with est/smoke-tests.ps1) and correct it to
include the missing leading "t" in the path.
In @.squad/agents/starkiller/history.md:
- Line 179: Update the recorded script path in the history entry: replace the
incorrect `est/smoke-tests.ps1` reference with the correct
`test/smoke-tests.ps1` in the text at the entry containing
"Designed/implemented/validated est/smoke-tests.ps1 (88 cases...)" so the
README/history accurately points to `test/smoke-tests.ps1`.
In `@test/smoke-tests.ps1`:
- Around line 160-168: Add a fail-fast check after discovery that aborts the
script when no specs were found: if $Specs.Count equals 0 (or $totalCases equals
0) call Write-Error with a clear message referencing the expected sample spec
directories (e.g., "No JSON specs found in openapi/v*/ directories") and then
exit with a non-zero code (exit 1) so CI fails; place this check right after
$Specs and $totalCases are computed and before the informational Write-Host
block that currently prints counts.
- Around line 193-229: Persist the generate/compile logs to per-case files under
the artifact directory instead of only keeping them in memory: create a per-case
output folder (using the existing $OutputDir, $spec.Version and $mode or
$caseLabel) before invoking & $Cli and & $ZigPath, write $genOutput to a file
(e.g., generate.log) immediately after capture and write $testOutput to a file
(e.g., compile.log) after capture, and update the results.Add entries to
reference those log file paths (and include them in the Output field) so
failures upload meaningful artifacts; ensure directory creation uses the same
$outParent / OutputDir logic so all logs land under test/output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 082c1c51-4770-4a15-b4dc-72bc218643a0
📒 Files selected for processing (10)
.github/workflows/ci.yml.gitignore.squad/agents/fenster/history.md.squad/agents/juno/history.md.squad/agents/lando/history.md.squad/agents/scribe/history.md.squad/agents/starkiller/history.md.squad/decisions.mdREADME.mdtest/smoke-tests.ps1
Comment on lines
+111
to
+112
| - est/smoke-tests.ps1 resolves repo root via $PSScriptRoot/.. and Set-Locations there, so it works from any CWD on PowerShell 7 (Windows + Linux + macOS GitHub runners). | ||
| - Uses Get-Command zig for tooling discovery, prints zig version, then builds once with zig build -Doptimize=ReleaseFast. Executable path uses \True to pick openapi2zig.exe vs openapi2zig. |
Contributor
There was a problem hiding this comment.
Clean up corrupted command/path text in the implementation notes.
Several lines have encoding/typing corruption (Line 111, Line 112, Line 114, Line 117, Line 129), which makes the notes hard to trust.
Suggested fix
-- - est/smoke-tests.ps1 resolves repo root via $PSScriptRoot/.. and Set-Locations there, so it works from any CWD on PowerShell 7 (Windows + Linux + macOS GitHub runners).
+- - test/smoke-tests.ps1 resolves repo root via $PSScriptRoot/.. and Set-Location there, so it works from any CWD on PowerShell 7 (Windows + Linux + macOS GitHub runners).
-- - Uses Get-Command zig for tooling discovery, prints zig version, then builds once with zig build -Doptimize=ReleaseFast. Executable path uses \True to pick openapi2zig.exe vs openapi2zig.
+- - Uses Get-Command zig for tooling discovery, prints zig version, then builds once with zig build -Doptimize=ReleaseFast. Executable path uses `$IsWindows` to pick openapi2zig.exe vs openapi2zig.
-- - Output layout: est/output/<version>/<basename>__<mode>.zig. Double underscore prevents collision when basenames already contain a dash. Cleaned each run unless -KeepOutput.
+- - Output layout: test/output/<version>/<basename>__<mode>.zig. Double underscore prevents collision when basenames already contain a dash. Cleaned each run unless -KeepOutput.
-- - Continues past failures, captures stdout+stderr per case via 2>&1, prints a final pass/fail/skip summary, and lists failing cases with phase (generate vs compile) before �xit 1.
+- - Continues past failures, captures stdout+stderr per case via 2>&1, prints a final pass/fail/skip summary, and lists failing cases with phase (generate vs compile) before exit 1.
-- - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.
+- - Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.Also applies to: 114-117, 129-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.squad/agents/fenster/history.md around lines 111 - 112, The implementation
notes contain corrupted command/path text; open .squad/agents/fenster/history.md
and fix the mangled lines referencing test/smoke-tests.ps1, $PSScriptRoot,
Set-Location, Get-Command zig, the zig build command (-Doptimize=ReleaseFast)
and the executable selection token (openapi2zig.exe vs openapi2zig or the flag
used to select it), restoring the original verbatim shell/PowerShell snippets
and paths so commands are accurate and consistent (e.g., "test/smoke-tests.ps1
resolves repo root via $PSScriptRoot/.. and Set-Location", "Get-Command zig",
"zig build -Doptimize=ReleaseFast", and the correct token used to choose
openapi2zig.exe). Ensure encoding/escaping is correct so backslashes and flags
render literally.
Comment on lines
+76
to
+80
| **Scope:** User wants comprehensive smoke tests (`test/smoke-tests.ps1`) to generate + compile all 24 OpenAPI specs, integrated into PR verification. | ||
|
|
||
| **Key findings:** | ||
| - **24 specs available** across v2.0 (8), v3.0 (9), v3.1 (2), v3.2 (2); exclude json-schema/ utilities | ||
| - **Current test coverage:** Only v2.0 + v3.0 petstore (2 specs); too narrow |
Contributor
There was a problem hiding this comment.
Reconcile the documented spec counts.
This history entry currently says the harness targets 24 specs, breaks them down as 8+9+2+2, and later says 22 specs shipped. Please align those numbers so the recorded coverage is unambiguous.
Also applies to: 127-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.squad/agents/juno/history.md around lines 76 - 80, Update the history entry
text under the "Scope" and "Key findings" sections so the spec counts are
consistent: either change the initial "24 specs" and the breakdown "8+9+2+2" to
match "22 specs shipped", or update "22 specs shipped" to "24 specs shipped" and
keep the 8+9+2+2 breakdown; ensure both the summary line and the later sentence
(the occurrence referenced at lines 127-129) use the same total and breakdown
values so the documented coverage is unambiguous.
| ### 2026-04-30T16:31:22.685+02:00 — Smoke Test Implementation Review (APPROVE) | ||
|
|
||
| ## Key Architecture | ||
| - Reviewed Fenster's est/smoke-tests.ps1 + .gitignore and Juno's CI + README changes against the approved plan. |
Contributor
There was a problem hiding this comment.
Remove control-character corruption in history text.
Line 97/Line 114 drop the leading t in test/smoke-tests.ps1, and Line 101 contains a corrupted refAllDeclsRecursive token.
Suggested fix
-- Reviewed Fenster's est/smoke-tests.ps1 + .gitignore and Juno's CI + README changes against the approved plan.
+- Reviewed Fenster's test/smoke-tests.ps1 + .gitignore and Juno's CI + README changes against the approved plan.
-- - Known limitation (acknowledged in plan): zig test relies on Zig's lazy analysis; deeply unused decls may escape compile-check. Acceptable for v1; can harden later with explicit
efAllDeclsRecursive wrapper if generator gaps slip through.
+- Known limitation (acknowledged in plan): zig test relies on Zig's lazy analysis; deeply unused decls may escape compile-check. Acceptable for v1; can harden later with an explicit refAllDeclsRecursive wrapper if generator gaps slip through.
-- - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.
+- - Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.Also applies to: 101-101, 114-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.squad/agents/lando/history.md at line 97, Open the
.squad/agents/lando/history.md file and remove the stray control characters that
are corrupting text; specifically restore the missing leading "t" in occurrences
of "test/smoke-tests.ps1" (replace any "est/smoke-tests.ps1" instances) and
correct the corrupted token "refAllDeclsRecursive" where it appears; ensure you
search for and replace any invisible control characters around those tokens so
the file contains the exact strings "test/smoke-tests.ps1" and
"refAllDeclsRecursive" (check the lines previously noted and any other
occurrences) and save.
| - History summarization gate checked; no history file met the 15360-byte threshold. | ||
|
|
||
| ## 2026-04-30 — Smoke-test harness shipped | ||
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
Contributor
There was a problem hiding this comment.
Fix malformed script path in the shipped note.
Line 27 shows est/smoke-tests.ps1 (missing leading t), which makes the record inaccurate.
Suggested fix
-- Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.
+- Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. | |
| - Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.squad/agents/scribe/history.md at line 27, Update the malformed script path
string "est/smoke-tests.ps1" in .squad/agents/scribe/history.md to the correct
path "test/smoke-tests.ps1" so the shipped note accurately reflects the script
name; locate the line containing the incorrect token (the phrase with
est/smoke-tests.ps1) and correct it to include the missing leading "t" in the
path.
| *To be updated as the team works.* | ||
|
|
||
| ## 2026-04-30 — Smoke-test harness shipped | ||
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
Contributor
There was a problem hiding this comment.
Correct the smoke-test script path typo.
Line 179 uses est/smoke-tests.ps1; it should be test/smoke-tests.ps1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.squad/agents/starkiller/history.md at line 179, Update the recorded script
path in the history entry: replace the incorrect `est/smoke-tests.ps1` reference
with the correct `test/smoke-tests.ps1` in the text at the entry containing
"Designed/implemented/validated est/smoke-tests.ps1 (88 cases...)" so the
README/history accurately points to `test/smoke-tests.ps1`.
Comment on lines
+160
to
+168
| $Specs = $Specs | Sort-Object Rel | ||
| $totalCases = $Specs.Count * $Modes.Count | ||
|
|
||
| Write-Host "" | ||
| Write-Host "Discovered $($Specs.Count) JSON specs across $($IncludedDirs.Count) directories" | ||
| Write-Host "Resource-wrapper modes: $($Modes -join ', ')" | ||
| Write-Host "Total cases to run: $totalCases (sequential)" | ||
| Write-Host "Skipped YAML files: $SkippedYaml (unsupported by generator)" | ||
| Write-Host "Output directory: $OutputDir" |
Contributor
There was a problem hiding this comment.
Fail fast when discovery finds zero specs.
Right now a bad path/layout change can make the smoke job pass without testing anything. For CI that is a false green on the exact sample set this harness is meant to validate.
Suggested fix
$Specs = $Specs | Sort-Object Rel
+if ($Specs.Count -eq 0) {
+ Write-Host "ERROR: no JSON specs matched '$Filter' under $($IncludedDirs -join ', ')." -ForegroundColor Red
+ exit 1
+}
$totalCases = $Specs.Count * $Modes.CountBased on learnings: Sample specifications in openapi/v?.?/ directory must be used for testing and validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke-tests.ps1` around lines 160 - 168, Add a fail-fast check after
discovery that aborts the script when no specs were found: if $Specs.Count
equals 0 (or $totalCases equals 0) call Write-Error with a clear message
referencing the expected sample spec directories (e.g., "No JSON specs found in
openapi/v*/ directories") and then exit with a non-zero code (exit 1) so CI
fails; place this check right after $Specs and $totalCases are computed and
before the informational Write-Host block that currently prints counts.
Comment on lines
+193
to
+229
| $outFile = Join-Path $OutputDir (Join-Path $spec.Version ("{0}__{1}.zig" -f $spec.Base, $mode)) | ||
| $outParent = Split-Path -Parent $outFile | ||
| if (-not (Test-Path $outParent)) { New-Item -ItemType Directory -Path $outParent -Force | Out-Null } | ||
|
|
||
| # ---- Generate ---- | ||
| $genOutput = & $Cli generate -i $spec.Abs -o $outFile --resource-wrappers $mode 2>&1 | ||
| $genExit = $LASTEXITCODE | ||
| if ($genExit -ne 0) { | ||
| Write-Host "FAIL (generate) $caseLabel" -ForegroundColor Red | ||
| Write-Host ($genOutput | Out-String) | ||
| $results.Add([pscustomobject]@{ | ||
| Spec = $spec.Rel; Mode = $mode; Status = "fail"; Phase = "generate" | ||
| ExitCode = $genExit; Output = ($genOutput | Out-String) | ||
| }) | ||
| continue | ||
| } | ||
|
|
||
| if (-not (Test-Path $outFile)) { | ||
| Write-Host "FAIL (generate produced no file) $caseLabel" -ForegroundColor Red | ||
| $results.Add([pscustomobject]@{ | ||
| Spec = $spec.Rel; Mode = $mode; Status = "fail"; Phase = "generate" | ||
| ExitCode = 0; Output = "no output file" | ||
| }) | ||
| continue | ||
| } | ||
|
|
||
| # ---- Compile-check ---- | ||
| # Generated files are library modules (no main()), so use `zig test`, | ||
| # which compiles them and reports "All 0 tests passed" on success. | ||
| $testOutput = & $ZigPath test $outFile 2>&1 | ||
| $testExit = $LASTEXITCODE | ||
| if ($testExit -ne 0) { | ||
| Write-Host "FAIL (compile) $caseLabel" -ForegroundColor Red | ||
| Write-Host ($testOutput | Out-String) | ||
| $results.Add([pscustomobject]@{ | ||
| Spec = $spec.Rel; Mode = $mode; Status = "fail"; Phase = "compile" | ||
| ExitCode = $testExit; Output = ($testOutput | Out-String) |
Contributor
There was a problem hiding this comment.
Persist per-case command logs under test/output/.
The artifact job uploads test/output/, but the generate/compile diagnostics are only printed to the console and kept in memory. A generate failure can therefore upload no useful evidence, and a compile failure uploads only the generated Zig file without the actual compiler output.
Suggested fix
$outFile = Join-Path $OutputDir (Join-Path $spec.Version ("{0}__{1}.zig" -f $spec.Base, $mode))
+ $caseStem = "{0}__{1}" -f $spec.Base, $mode
+ $genLog = Join-Path $OutputDir (Join-Path $spec.Version ("{0}.generate.log" -f $caseStem))
+ $testLog = Join-Path $OutputDir (Join-Path $spec.Version ("{0}.compile.log" -f $caseStem))
$outParent = Split-Path -Parent $outFile
if (-not (Test-Path $outParent)) { New-Item -ItemType Directory -Path $outParent -Force | Out-Null }
# ---- Generate ----
$genOutput = & $Cli generate -i $spec.Abs -o $outFile --resource-wrappers $mode 2>&1
$genExit = $LASTEXITCODE
+ $genOutput | Out-File -FilePath $genLog -Encoding utf8
if ($genExit -ne 0) {
Write-Host "FAIL (generate) $caseLabel" -ForegroundColor Red
Write-Host ($genOutput | Out-String)
$results.Add([pscustomobject]@{
Spec = $spec.Rel; Mode = $mode; Status = "fail"; Phase = "generate"
@@
# ---- Compile-check ----
# Generated files are library modules (no main()), so use `zig test`,
# which compiles them and reports "All 0 tests passed" on success.
$testOutput = & $ZigPath test $outFile 2>&1
$testExit = $LASTEXITCODE
+ $testOutput | Out-File -FilePath $testLog -Encoding utf8
if ($testExit -ne 0) {
Write-Host "FAIL (compile) $caseLabel" -ForegroundColor Red
Write-Host ($testOutput | Out-String)
$results.Add([pscustomobject]@{
Spec = $spec.Rel; Mode = $mode; Status = "fail"; Phase = "compile"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke-tests.ps1` around lines 193 - 229, Persist the generate/compile
logs to per-case files under the artifact directory instead of only keeping them
in memory: create a per-case output folder (using the existing $OutputDir,
$spec.Version and $mode or $caseLabel) before invoking & $Cli and & $ZigPath,
write $genOutput to a file (e.g., generate.log) immediately after capture and
write $testOutput to a file (e.g., compile.log) after capture, and update the
results.Add entries to reference those log file paths (and include them in the
Output field) so failures upload meaningful artifacts; ensure directory creation
uses the same $outParent / OutputDir logic so all logs land under test/output.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a cross-platform smoke-test harness to exercise openapi2zig code generation across the repository’s sample OpenAPI/Swagger specs (and wrapper modes), then compile-check the generated Zig outputs, with CI integration and supporting documentation.
Changes:
- Added
test/smoke-tests.ps1to discover JSON specs, generate outputs per wrapper mode, and compile-check withzig test, including a denylist for known generator gaps. - Wired the smoke script into the existing
smoke-testsGitHub Actions job and configured artifact upload oftest/output/on failure. - Documented the workflow in README and recorded the decision/process notes; ignored
test/output/in git.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
test/smoke-tests.ps1 |
Implements the smoke-test runner (spec discovery, generation, compile-check, denylist, summary). |
README.md |
Documents how to run smoke tests locally and describes CI behavior. |
.github/workflows/ci.yml |
Adds a CI step to run the smoke script and upload outputs on failure. |
.gitignore |
Ignores test/output/ generated by the smoke tests. |
.squad/decisions.md |
Records the smoke-test harness decision and contract. |
.squad/agents/*/history.md |
Adds session history notes about the smoke-test work. |
Comment on lines
+63
to
+74
| # Keep this list small and explicit. Each entry is a generator gap to fix. | ||
| # This list intentionally starts empty; populate only when CI signal demands | ||
| # it and a tracking issue exists. | ||
| # --------------------------------------------------------------------------- | ||
| $Denylist = @( | ||
| # openapi/v3.0/ingram-micro.json fails the compile phase in every | ||
| # resource-wrapper mode (none, tags, paths, hybrid) because the unified | ||
| # model generator emits duplicate `pub const X = struct` declarations for | ||
| # several shared schemas. Denylisted across all modes via Mode="*" until | ||
| # the unified model generator dedupes shared/nested type emissions. | ||
| @{ Spec = "openapi/v3.0/ingram-micro.json"; Mode = "*"; Reason = "duplicate `pub const` emissions from unified model generator (all wrapper modes)" } | ||
| ) |
There was a problem hiding this comment.
The comment says the denylist “intentionally starts empty”, but the script now ships with an ingram-micro.json entry. Please update the comment to reflect the current state/policy (e.g., “keep this list small; currently includes …”).
| ### 2026-04-30: Smoke-test harness for code generation | ||
|
|
||
| - **Decision:** Add a PowerShell-driven smoke-test harness that runs the full spec × resource-wrapper matrix and validates that generated Zig compiles, integrated into the existing `smoke-tests` CI job. | ||
| - **Script contract (`test/smoke-tests.ps1`, owned by Fenster):** Lives at `test/smoke-tests.ps1`; resolves repo root from `$PSCommandPath` (CWD-independent); builds once with `-Doptimize=ReleaseFast`; enumerates JSON specs under `openapi/v2.0`, `v3.0`, `v3.1`, `v3.2`; skips `openapi/json-schema/` and counts YAML files as a single skip total; iterates 22 specs × 4 wrapper modes (`none`, `tags`, `paths`, `hybrid`) = 88 cases sequentially; writes outputs to `test/output/<version>/<basename>__<mode>.zig` (cleared per run unless `-KeepOutput`); exposes `-Filter` glob and `-Modes` selector for focused runs; exits non-zero only when non-denylisted failures occur. |
There was a problem hiding this comment.
This decision text says the smoke script “builds once with -Doptimize=ReleaseFast”, but the script actually accepts -Optimize and only defaults to ReleaseFast. Update the decision record to avoid implying the optimize mode is fixed.
Suggested change
| - **Script contract (`test/smoke-tests.ps1`, owned by Fenster):** Lives at `test/smoke-tests.ps1`; resolves repo root from `$PSCommandPath` (CWD-independent); builds once with `-Doptimize=ReleaseFast`; enumerates JSON specs under `openapi/v2.0`, `v3.0`, `v3.1`, `v3.2`; skips `openapi/json-schema/` and counts YAML files as a single skip total; iterates 22 specs × 4 wrapper modes (`none`, `tags`, `paths`, `hybrid`) = 88 cases sequentially; writes outputs to `test/output/<version>/<basename>__<mode>.zig` (cleared per run unless `-KeepOutput`); exposes `-Filter` glob and `-Modes` selector for focused runs; exits non-zero only when non-denylisted failures occur. | |
| - **Script contract (`test/smoke-tests.ps1`, owned by Fenster):** Lives at `test/smoke-tests.ps1`; resolves repo root from `$PSCommandPath` (CWD-independent); builds once using the selected `-Optimize` mode (default `ReleaseFast`); enumerates JSON specs under `openapi/v2.0`, `v3.0`, `v3.1`, `v3.2`; skips `openapi/json-schema/` and counts YAML files as a single skip total; iterates 22 specs × 4 wrapper modes (`none`, `tags`, `paths`, `hybrid`) = 88 cases sequentially; writes outputs to `test/output/<version>/<basename>__<mode>.zig` (cleared per run unless `-KeepOutput`); exposes `-Filter` glob and `-Modes` selector for focused runs; exits non-zero only when non-denylisted failures occur. |
Comment on lines
+5
to
+15
| **Tech Stack:** Zig v0.15.2, OpenAPI v3.0, Swagger v2.0 | ||
| **Architecture:** Unified converter pattern (parse → normalize → generate) | ||
|
|
||
| ## Key Architecture | ||
|
|
||
| - **Specs supported:** Swagger v2.0, OpenAPI v3.0 | ||
| - **Parsing:** Version-specific models in `src/models/v2.0/` and `src/models/v3.0/` | ||
| - **Converters:** `src/generators/converters/` transforms version-specific → unified document | ||
| - **Generation:** `src/generators/unified/` generates Zig code from unified representation | ||
| - **Memory model:** Every dynamic struct has `deinit(allocator)`. **Always use `defer parsed.deinit(allocator)`.** | ||
| - **Testing:** xUnit-style with `test_utils.createTestAllocator()`, test against both v2.0 and v3.0 |
There was a problem hiding this comment.
This project context is stale: the repo requires Zig 0.16.0+ (see build.zig.zon minimum_zig_version and CI ZIG_VERSION), and the generator supports OpenAPI 3.1/3.2 in addition to 3.0. Please update these bullets to match current repo reality.
Suggested change
| **Tech Stack:** Zig v0.15.2, OpenAPI v3.0, Swagger v2.0 | |
| **Architecture:** Unified converter pattern (parse → normalize → generate) | |
| ## Key Architecture | |
| - **Specs supported:** Swagger v2.0, OpenAPI v3.0 | |
| - **Parsing:** Version-specific models in `src/models/v2.0/` and `src/models/v3.0/` | |
| - **Converters:** `src/generators/converters/` transforms version-specific → unified document | |
| - **Generation:** `src/generators/unified/` generates Zig code from unified representation | |
| - **Memory model:** Every dynamic struct has `deinit(allocator)`. **Always use `defer parsed.deinit(allocator)`.** | |
| - **Testing:** xUnit-style with `test_utils.createTestAllocator()`, test against both v2.0 and v3.0 | |
| **Tech Stack:** Zig v0.16.0+, OpenAPI v3.0/v3.1/v3.2, Swagger v2.0 | |
| **Architecture:** Unified converter pattern (parse → normalize → generate) | |
| ## Key Architecture | |
| - **Specs supported:** Swagger v2.0, OpenAPI v3.0/v3.1/v3.2 | |
| - **Parsing:** Version-specific models in `src/models/v2.0/` and `src/models/v3.0/` | |
| - **Converters:** `src/generators/converters/` transforms version-specific → unified document | |
| - **Generation:** `src/generators/unified/` generates Zig code from unified representation | |
| - **Memory model:** Every dynamic struct has `deinit(allocator)`. **Always use `defer parsed.deinit(allocator)`.** | |
| - **Testing:** xUnit-style with `test_utils.createTestAllocator()`, test against Swagger v2.0 and OpenAPI v3.x inputs |
| - History summarization gate checked; no history file met the 15360-byte threshold. | ||
|
|
||
| ## 2026-04-30 — Smoke-test harness shipped | ||
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
There was a problem hiding this comment.
This line contains a tab/control character in the script path (it renders as est/smoke-tests.ps1 / missing the leading t). Please replace it with plain test/smoke-tests.ps1 so the path is copy/pasteable.
Suggested change
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. | |
| - Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
Comment on lines
+111
to
+117
| - est/smoke-tests.ps1 resolves repo root via $PSScriptRoot/.. and Set-Locations there, so it works from any CWD on PowerShell 7 (Windows + Linux + macOS GitHub runners). | ||
| - Uses Get-Command zig for tooling discovery, prints zig version, then builds once with zig build -Doptimize=ReleaseFast. Executable path uses \True to pick openapi2zig.exe vs openapi2zig. | ||
| - Spec discovery: enumerates openapi/v2.0, openapi/v3.0, openapi/v3.1, openapi/v3.2 for *.json. YAML files are counted as skipped (visibility) but never run. openapi/json-schema/ is excluded by not being in the include list. | ||
| - Output layout: est/output/<version>/<basename>__<mode>.zig. Double underscore prevents collision when basenames already contain a dash. Cleaned each run unless -KeepOutput. | ||
| - Compile check uses zig test <output> (NOT zig run) because generated files are library modules. Confirmed locally: emits All 0 tests passed and exits 0. | ||
| - Denylist is a list of @{ Spec; Mode; Reason } hashtables; * wildcards both fields. Currently empty — keep it that way until CI signal proves a gap. | ||
| - Continues past failures, captures stdout+stderr per case via 2>&1, prints a final pass/fail/skip summary, and lists failing cases with phase (generate vs compile) before xit 1. |
There was a problem hiding this comment.
This section contains multiple copy/paste issues that make it inaccurate/confusing (path rendered as est/..., references $PSScriptRoot/Set-Locations though the script uses $PSCommandPath/Set-Location, \True instead of $IsWindows, “denylist … currently empty” despite an ingram-micro entry, and �xit 1). Please clean these up so the history reflects the actual implementation.
Comment on lines
+97
to
+101
| - Reviewed Fenster's est/smoke-tests.ps1 + .gitignore and Juno's CI + README changes against the approved plan. | ||
| - Implementation matches all four confirmed decisions (temp denylist, all 4 wrapper modes, keep petstore harness, output under test/output/). | ||
| - Cross-platform handling is sound: repo-root via $PSCommandPath, $IsWindows for exe suffix, slash normalization after Resolve-Path -Relative, `shell: pwsh` in CI. | ||
| - CI scoping is correct: appended to existing smoke-tests job; artifact upload guarded by if: failure(); petstore harness preserved. | ||
| - Known limitation (acknowledged in plan): zig test relies on Zig's lazy analysis; deeply unused decls may escape compile-check. Acceptable for v1; can harden later with explicit efAllDeclsRecursive wrapper if generator gaps slip through. |
There was a problem hiding this comment.
This line appears to contain a stray tab/control character inside the path ( est/... renders as est), and the word refAllDeclsRecursive is also corrupted (missing the leading r). Please replace the control characters with plain text so the history entry stays searchable and renders correctly.
Comment on lines
+76
to
+79
| **Scope:** User wants comprehensive smoke tests (`test/smoke-tests.ps1`) to generate + compile all 24 OpenAPI specs, integrated into PR verification. | ||
|
|
||
| **Key findings:** | ||
| - **24 specs available** across v2.0 (8), v3.0 (9), v3.1 (2), v3.2 (2); exclude json-schema/ utilities |
There was a problem hiding this comment.
Spec counts here are incorrect: the repo currently has 22 eligible JSON API specs (v2.0: 8, v3.0: 10 incl. ingram-micro.json, v3.1: 2, v3.2: 2). Please update this line so future readers don’t rely on the wrong totals.
Suggested change
| **Scope:** User wants comprehensive smoke tests (`test/smoke-tests.ps1`) to generate + compile all 24 OpenAPI specs, integrated into PR verification. | |
| **Key findings:** | |
| - **24 specs available** across v2.0 (8), v3.0 (9), v3.1 (2), v3.2 (2); exclude json-schema/ utilities | |
| **Scope:** User wants comprehensive smoke tests (`test/smoke-tests.ps1`) to generate + compile all 22 eligible JSON API specs, integrated into PR verification. | |
| **Key findings:** | |
| - **22 eligible JSON API specs available** across v2.0 (8), v3.0 (10, including `ingram-micro.json`), v3.1 (2), v3.2 (2); exclude json-schema/ utilities |
| - **No UX issues:** Script can use existing CLI as-is | ||
|
|
||
| **Recommendation (MVP):** | ||
| 1. Create `test/smoke-tests.ps1` (~250 lines): discover specs, generate sequentially, compile each with `zig run`, report per-spec pass/fail |
There was a problem hiding this comment.
This recommendation says to compile generated files with zig run, but generated outputs are library modules without a main(); this will fail. The approach used elsewhere in the repo is zig test <generated.zig> (or a harness that refAllDecls), so the note should be corrected.
Suggested change
| 1. Create `test/smoke-tests.ps1` (~250 lines): discover specs, generate sequentially, compile each with `zig run`, report per-spec pass/fail | |
| 1. Create `test/smoke-tests.ps1` (~250 lines): discover specs, generate sequentially, validate each generated module with `zig test`, report per-spec pass/fail (or use a small harness that `refAllDecls` when needed) |
| *To be updated as the team works.* | ||
|
|
||
| ## 2026-04-30 — Smoke-test harness shipped | ||
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
There was a problem hiding this comment.
This line contains a tab/control character in the script path (it renders as est/smoke-tests.ps1 / missing the leading t). Please replace it with plain test/smoke-tests.ps1 so the path is copy/pasteable.
Suggested change
| - Designed/implemented/validated est/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. | |
| - Designed/implemented/validated test/smoke-tests.ps1 (88 cases: 22 specs × 4 wrapper modes), CI job updated with failure-only artifact upload, README documented. |
Comment on lines
+149
to
+150
| # Pattern match against the slash-normalized relative path. | ||
| if ($relPath -notlike "*$Filter*" -and ($Filter -ne "*")) { continue } |
There was a problem hiding this comment.
-Filter is documented as a wildcard pattern matched against the relative spec path, but the implementation wraps the user input in *...* and uses -notlike, which effectively turns it into a substring match and can change wildcard semantics. Consider switching to $relPath -like $Filter (after slash normalization) or update the help text to describe the current behavior precisely.
Suggested change
| # Pattern match against the slash-normalized relative path. | |
| if ($relPath -notlike "*$Filter*" -and ($Filter -ne "*")) { continue } | |
| # Match the slash-normalized relative path against the user-supplied wildcard pattern. | |
| if (($Filter -ne "*") -and ($relPath -notlike $Filter)) { continue } |
christianhelle
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Documentation