Improve Smoke Tests execution time

[christianhelle]

This pull request updates the smoke test infrastructure and related configuration to improve reliability and maintainability, especially for batch testing scenarios. The most important changes are grouped below by theme:

Smoke Test Infrastructure Improvements:

• Added a ThrottleLimit parameter to the smoke-tests.bat script to control parallelism, replacing the previous PARALLEL variable. This helps prevent resource exhaustion during batch test runs. [1] [2]
• Refactored the batch script to modularize code generation, solution building, and cleanup steps, improving readability and extensibility. New functions include :generate, :generate_from_settings, :build_solution, and :clean_generated_code.
• Updated the build step to use .NET 9.0 and release configuration for publishing Refitter, ensuring compatibility with the latest .NET version.

Configuration Changes for Smoke Tests:

• Added a conditional property group in Directory.Build.props to disable the Refit source generator and warnings-as-errors during smoke tests, preventing filename-too-long errors when many interfaces are generated.

Test Settings Updates:

• Updated multiple-sources.refitter to specify a custom namespace and output folder for generated code, improving organization of test artifacts.

Summary by CodeRabbit

• Tests

  • Enhanced smoke test infrastructure with throttle control for managing batch test execution.
  • Refactored test execution pipeline with improved phase-based organization and modular test helpers.

• Chores

  • Updated configuration format with namespace and output folder settings.
  • Improved build configuration to optimize smoke test performance and reduce build-related errors.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors smoke testing infrastructure by introducing throttle-based execution control, modularizing test orchestration functions, and adding phase-structured build/generate workflows. MSBuild properties are configured to handle filename length constraints during batch testing. A settings file configuration is added for multi-source test scenarios.

Changes

Cohort / File(s)	Summary
Build Configuration test/ConsoleApp/Directory.Build.props	Adds conditional MSBuild property group that disables Refit source generator and warning-as-errors treatment when smoke tests are enabled, preventing filename-too-long errors.
Test Configuration test/multiple-sources.refitter	Adds namespace and outputFolder properties to specify test output location and namespace for multi-source code generation scenarios.
Batch Script Refactoring test/smoke-tests.bat	Replaces parallel execution with throttle-limit control; restructures into phases 0-5 for standardized variant and netCore variant generation/building; renames generate_and_build to generate; unifies file path and namespace computation logic; updates cleanup and invocation patterns.
PowerShell Script Refactoring test/smoke-tests.ps1	Decomposes monolithic GenerateAndBuild function into modular helpers (GetProcessPath, StartRefitter, BuildSolution, CleanGeneratedCode, RunGenerationTasks, GenerateFromSettingsFile); simplifies RunTests signature; implements phase-based orchestration for restoration, settings-file tests, variants, and URL tests.

Sequence Diagram(s)

sequenceDiagram
    participant TestScript as Test Orchestration
    participant Helper as Helper Functions
    participant Refitter as Refitter/Docker
    participant Build as dotnet build
    participant FileSystem as Generated Code

    TestScript->>TestScript: Phase 0: Pre-restore
    TestScript->>Build: dotnet build (restore)
    Build->>FileSystem: Generate dependencies

    TestScript->>TestScript: Phase 1-2: Standard & V3.1 Variants
    loop For each variant
        TestScript->>Helper: StartRefitter(variant args)
        Helper->>Refitter: Execute generation
        Refitter->>FileSystem: Write generated code
    end
    TestScript->>Build: Build generated code

    TestScript->>TestScript: Phase 3: netCore Variants
    loop For each netCore variant
        TestScript->>Helper: StartRefitter(variant args)
        Helper->>Refitter: Execute generation
        Refitter->>FileSystem: Write generated code
    end
    TestScript->>Build: Build netCore code

    TestScript->>TestScript: Phase 4: URL-based Tests
    TestScript->>Helper: GenerateFromSettingsFile()
    Helper->>Refitter: Run from settings
    Refitter->>FileSystem: Generate from remote spec
    TestScript->>Build: Build generated code

    TestScript->>FileSystem: CleanGeneratedCode()
    FileSystem->>FileSystem: Remove artifacts

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• Update Smoke Tests workflow trigger #791: Modifies CI workflows to execute the refactored smoke-tests.ps1 script, directly consuming these script changes.
• Add .NET 10 support #822: Also modifies test/ConsoleApp/Directory.Build.props and smoke test scripts together, affecting MSBuild and test orchestration behavior.
• Fix match path on cmd prompt #719: Modifies smoke test scripts with related test orchestration function changes and batch file refactoring.

Suggested labels

enhancement

Poem

🐰 Phases dance in thrifty rows,
No parallel, but throttle flows,
Modular helpers, clean and neat,
Build and generate, tests compete,
The scripts hop forward, swift and true! 🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Improve Smoke Tests execution time' directly aligns with the main objective of the PR, which is to optimize smoke test infrastructure through refactoring and parallelism control.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch smoke-tests

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

test/smoke-tests.ps1 (1)

10-13: $Parallel is currently inert despite being accepted.

If you keep it for compatibility, emit a deprecation warning so callers know it has no effect.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/smoke-tests.ps1` around lines 10 - 13, The $Parallel parameter is
accepted but does nothing; add a deprecation warning when callers pass it so
they know it has no effect. After parameter binding (e.g. at the top of the
script or inside the main entry function), detect if the $Parallel parameter was
supplied (check PSBoundParameters.ContainsKey('Parallel') or test
$PSBoundParameters['Parallel']) and emit a clear deprecation warning via
Write-Warning stating that "$Parallel is deprecated and has no effect" and
suggesting the preferred behavior; keep the parameter for backward compatibility
but ensure the warning runs only when it was explicitly provided.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/smoke-tests.bat`:
- Around line 5-16: The -throttlelimit option currently only updates
THROTTLE_LIMIT in the parse_args block but is never used; either remove the
option parsing or actually apply THROTTLE_LIMIT when running tasks. Fix by
wiring THROTTLE_LIMIT into the task execution loop (where parallel jobs are
started) so it limits concurrent tasks to %THROTTLE_LIMIT% (e.g., track running
background jobs and wait when count >= %THROTTLE_LIMIT%), or if you prefer to
deprecate it, remove the -throttlelimit handling from parse_args and any
help/echo references to avoid misleading users; key symbols: THROTTLE_LIMIT,
parse_args, and the -throttlelimit option.

In `@test/smoke-tests.ps1`:
- Around line 207-221: The Start-Process invocations that run dotnet
publish/restore currently wait but don't check ExitCode, so failures are
ignored; update each invocation (the Start-Process calls that run "dotnet" for
publish and the three restore calls) to capture the returned process object,
Wait-Process, then inspect its ExitCode and throw/Write-Error including the
command and ExitCode (and optionally stderr) on non-zero to fail fast; do the
same for the refitter --version run that sets $p so all critical Start-Process
usages validate ExitCode and abort on failure.
- Around line 397-403: The generate-only invocation builds $customNameCmd by
concatenating $processPath directly, which breaks when Docker mode sets
$processPath to "docker"; update the construction of $customNameCmd to use the
same Docker-aware invocation used elsewhere in the script: if $processPath -eq
"docker" (or the script's Docker flag is set) compose the full docker run
command (including the image variable used elsewhere, bind mounts and any docker
run args) and then append $customNameSpec, --namespace, --output and
$customNameArgs; otherwise continue to build the normal local invocation. Ensure
you reference and reuse the existing Docker image/run variables or helper
function used by other invocations rather than hardcoding values so
$customNameCmd works in both Docker and non-Docker modes.

---

Nitpick comments:
In `@test/smoke-tests.ps1`:
- Around line 10-13: The $Parallel parameter is accepted but does nothing; add a
deprecation warning when callers pass it so they know it has no effect. After
parameter binding (e.g. at the top of the script or inside the main entry
function), detect if the $Parallel parameter was supplied (check
PSBoundParameters.ContainsKey('Parallel') or test
$PSBoundParameters['Parallel']) and emit a clear deprecation warning via
Write-Warning stating that "$Parallel is deprecated and has no effect" and
suggesting the preferred behavior; keep the parameter for backward compatibility
but ensure the warning runs only when it was explicitly provided.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbdab25 and fe44f30.

📒 Files selected for processing (4)

• test/ConsoleApp/Directory.Build.props
• test/multiple-sources.refitter
• test/smoke-tests.bat
• test/smoke-tests.ps1

[coderabbitai]

⚠️ Potential issue | 🟠 Major

-throttlelimit is parsed but never applied.

Right now this option only updates echoed output and does not affect task execution. Either wire it into real throttled execution or remove/deprecate it to avoid misleading behavior.

Also applies to: 37-37

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/smoke-tests.bat` around lines 5 - 16, The -throttlelimit option
currently only updates THROTTLE_LIMIT in the parse_args block but is never used;
either remove the option parsing or actually apply THROTTLE_LIMIT when running
tasks. Fix by wiring THROTTLE_LIMIT into the task execution loop (where parallel
jobs are started) so it limits concurrent tasks to %THROTTLE_LIMIT% (e.g., track
running background jobs and wait when count >= %THROTTLE_LIMIT%), or if you
prefer to deprecate it, remove the -throttlelimit handling from parse_args and
any help/echo references to avoid misleading users; key symbols: THROTTLE_LIMIT,
parse_args, and the -throttlelimit option.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fail fast on publish/restore failures.

These commands currently wait for completion but do not validate ExitCode, so the script can continue after a failed setup step.

Proposed fix

-        Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru | Wait-Process
+        $p = Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru
+        $p | Wait-Process
+        if ($p.ExitCode -ne 0) { throw "Publish failed: ../src/Refitter/Refitter.csproj" }

-    Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
-    Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
-    Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru | Wait-Process
+    $p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru
+    $p | Wait-Process
+    if ($p.ExitCode -ne 0) { throw "Restore failed: ./ConsoleApp/ConsoleApp.slnx" }
+    $p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru
+    $p | Wait-Process
+    if ($p.ExitCode -ne 0) { throw "Restore failed: ./ConsoleApp/ConsoleApp.Core.slnx" }
+    $p = Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru
+    $p | Wait-Process
+    if ($p.ExitCode -ne 0) { throw "Restore failed: ./Apizr/Sample.csproj" }

📝 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

	Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru | Wait-Process
	Write-Host "refitter --version"
	$process = Start-Process "./bin/refitter" `
	-Args " --version" `
	-NoNewWindow `
	-PassThru
	$process | Wait-Process
	if ($process.ExitCode -ne 0)
	{
	throw "Show version failed!"
	}
	$p = Start-Process "./bin/refitter" -Args "--version" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Show version failed!" }
	}
	GenerateAndBuild -format " " -namespace " " -outputPath "SwaggerPetstoreDirect.generated.cs" -args "--settings-file ./petstore.refitter" -buildFromSource $buildFromSource -useDocker $UseDocker
	GenerateAndBuild -format " " -namespace " " -args "--settings-file ./Apizr/petstore.apizr.refitter" -csproj "./Apizr/Sample.csproj" -buildFromSource $buildFromSource -useDocker $UseDocker
	GenerateAndBuild -format " " -namespace " " -args "--settings-file ./MultipleFiles/petstore.refitter" -csproj "MultipleFiles/Client/Client.csproj" -buildFromSource $buildFromSource -useDocker $UseDocker
	"v3.0", "v2.0" | ForEach-Object {
	$version = $_
	"json", "yaml" | ForEach-Object {
	$format = $_
	$filenames | ForEach-Object {
	$filename = "./OpenAPI/$version/$_.$format"
	$exists = Test-Path -Path $filename -PathType Leaf
	if ($exists -eq $true)
	# ==========================================
	# Phase 1: Pre-restore packages
	# ==========================================
	Write-Host "`r`n=== Pre-restoring packages ===`r`n"
	Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	$p = Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Publish failed: ../src/Refitter/Refitter.csproj" }
	Write-Host "refitter --version"
	$p = Start-Process "./bin/refitter" -Args "--version" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Show version failed!" }
	}
	# ==========================================
	# Phase 1: Pre-restore packages
	# ==========================================
	Write-Host "`r`n=== Pre-restoring packages ===`r`n"
	$p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Restore failed: ./ConsoleApp/ConsoleApp.slnx" }
	$p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Restore failed: ./ConsoleApp/ConsoleApp.Core.slnx" }
	$p = Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "Restore failed: ./Apizr/Sample.csproj" }

🧰 Tools

🪛 PSScriptAnalyzer (1.24.0)

[warning] 209-209: File 'smoke-tests.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.

(PSAvoidUsingWriteHost)

---

[warning] 218-218: File 'smoke-tests.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.

(PSAvoidUsingWriteHost)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/smoke-tests.ps1` around lines 207 - 221, The Start-Process invocations
that run dotnet publish/restore currently wait but don't check ExitCode, so
failures are ignored; update each invocation (the Start-Process calls that run
"dotnet" for publish and the three restore calls) to capture the returned
process object, Wait-Process, then inspect its ExitCode and throw/Write-Error
including the command and ExitCode (and optionally stderr) on non-zero to fail
fast; do the same for the refitter --version run that sets $p so all critical
Start-Process usages validate ExitCode and abort on failure.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Generate-only phase breaks in Docker mode.

This branch constructs $processPath ... directly. When Docker is enabled, $processPath is just docker, so the required Docker run prefix/image is skipped and generation fails.

Proposed fix

-    $customNameCmd = "$processPath $customNameSpec --namespace GenerateOnly.MultipleInterfacesWithCustomName --output $customNameOutput --no-logging $customNameArgs"
-    Write-Host $customNameCmd
-    Invoke-Expression $customNameCmd
-    if (-not (Test-Path $customNameOutput)) { throw "Generate-only test failed: MultipleInterfacesWithCustomName" }
+    $p = StartRefitter `
+        -arguments "$customNameSpec --namespace GenerateOnly.MultipleInterfacesWithCustomName --output $customNameOutput --no-logging $customNameArgs" `
+        -processPath $processPath `
+        -useDocker $UseDocker
+    $p | Wait-Process
+    if ($p.ExitCode -ne 0 -or -not (Test-Path $customNameOutput)) {
+        throw "Generate-only test failed: MultipleInterfacesWithCustomName"
+    }

🧰 Tools

🪛 PSScriptAnalyzer (1.24.0)

[warning] 402-402: Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.

(PSAvoidUsingInvokeExpression)

---

[warning] 401-401: File 'smoke-tests.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.

(PSAvoidUsingWriteHost)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/smoke-tests.ps1` around lines 397 - 403, The generate-only invocation
builds $customNameCmd by concatenating $processPath directly, which breaks when
Docker mode sets $processPath to "docker"; update the construction of
$customNameCmd to use the same Docker-aware invocation used elsewhere in the
script: if $processPath -eq "docker" (or the script's Docker flag is set)
compose the full docker run command (including the image variable used
elsewhere, bind mounts and any docker run args) and then append $customNameSpec,
--namespace, --output and $customNameArgs; otherwise continue to build the
normal local invocation. Ensure you reference and reuse the existing Docker
image/run variables or helper function used by other invocations rather than
hardcoding values so $customNameCmd works in both Docker and non-Docker modes.

[Copilot]

Pull request overview

This PR refactors the smoke test infrastructure to improve execution time and reliability by batching code generation and builds, rather than running a full generate-then-build cycle per variant. It also adds support for v3.1 OpenAPI specs, introduces modular helper functions, and configures the build system to avoid filename-too-long errors during batch builds.

Changes:

• Refactored smoke-tests.ps1 and smoke-tests.bat to separate code generation from builds (generate all → build once), added helper functions (GetProcessPath, BuildDockerPrefix, StartRefitter, GenerateFromSettingsFile, BuildSolution, CleanGeneratedCode, RunGenerationTasks), added v3.1 spec support, and replaced the PARALLEL variable with ThrottleLimit
• Added a conditional SmokeTest property group to ConsoleApp/Directory.Build.props to disable the Refit source generator and TreatWarningsAsErrors during batch smoke test runs
• Updated multiple-sources.refitter to specify namespace and outputFolder fields so it generates into a predictable location

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File	Description
test/smoke-tests.ps1	Major refactor: modular helper functions, phased test execution, batch generate-then-build
test/smoke-tests.bat	Equivalent refactor to PS1: modular subroutines, phased execution, adds v3.1 and ThrottleLimit
test/multiple-sources.refitter	Adds namespace and outputFolder for deterministic output location
test/ConsoleApp/Directory.Build.props	New conditional property group to disable source generator and warnings-as-errors during batch runs

[Copilot]

The $urlFormat variable is assigned at line 428 but is never used anywhere in the surrounding code block. It appears to be dead code left over from refactoring. It should either be removed or used as part of the output path to differentiate the JSON and YAML test artifacts.

Suggested change

$urlFormat = if ($url.EndsWith(".json")) { "json" } else { "yaml" }

[Copilot]

The Phase 4b "Generate-only" test uses Invoke-Expression to run the refitter command directly, but this does not correctly handle the Docker mode scenario. When $UseDocker is $true, $processPath is set to "docker" by GetProcessPath, and the constructed command "docker ./OpenAPI/v3.0/petstore.json --namespace ..." is missing the required run --rm -v ... Docker prefix that StartRefitter/BuildDockerPrefix would provide.

The function StartRefitter already handles the Docker/non-Docker distinction correctly. This block should be refactored to use StartRefitter (passing $processPath and -useDocker $UseDocker) instead of Invoke-Expression, which would also address the general recommendation against using Invoke-Expression.

Suggested change

	$customNameCmd = "$processPath $customNameSpec --namespace GenerateOnly.MultipleInterfacesWithCustomName --output $customNameOutput --no-logging $customNameArgs"
	Write-Host $customNameCmd
	Invoke-Expression $customNameCmd
	$customNameCmdArgs = "$customNameSpec --namespace GenerateOnly.MultipleInterfacesWithCustomName --output $customNameOutput --no-logging $customNameArgs"
	Write-Host "$processPath $customNameCmdArgs"
	StartRefitter -processPath $processPath -arguments $customNameCmdArgs -useDocker:$UseDocker

[Copilot]

The comment at line 382 says "Execute standard generation in parallel batches", but RunGenerationTasks is entirely sequential — it uses a plain for loop with Wait-Process after each task. The comment is misleading and should be updated to reflect that generation is sequential.

Suggested change

	# Execute standard generation in parallel batches
	# Execute standard generation tasks sequentially

[Copilot]

The THROTTLE_LIMIT variable is parsed from the command line arguments and echoed, but it is never actually used to control parallelism anywhere in the script. The PR description states this parameter "helps prevent resource exhaustion during batch test runs," but the batch script has no parallel execution logic that reads this variable. It should either be wired into a parallel execution mechanism or removed to avoid confusion.

[Copilot]

The dotnet publish command at line 207 is run via Start-Process ... | Wait-Process but its exit code is never checked. If the publish fails, the script will continue and attempt to run ./bin/refitter which won't exist, causing a confusing failure further down. The exit code should be checked here, consistent with how the following --version check works (lines 210-212).

Suggested change

	Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru | Wait-Process
	$publishProcess = Start-Process "dotnet" -Args "publish ../src/Refitter/Refitter.csproj -c Release -o bin -f net9.0" -NoNewWindow -PassThru
	$publishProcess | Wait-Process
	if ($publishProcess.ExitCode -ne 0) { throw "dotnet publish failed!" }

[Copilot]

The pre-restore commands at lines 219-221 do not check exit codes. If a restore fails, the build will fail with a less informative error. This is especially important because the build commands later use --no-restore. Consider capturing the returned process objects and checking their ExitCode, or at minimum using ThrowOnNativeFailure after each restore.

Suggested change

	Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru | Wait-Process
	$p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.slnx --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "dotnet restore ./ConsoleApp/ConsoleApp.slnx failed!" }
	$p = Start-Process "dotnet" -Args "restore ./ConsoleApp/ConsoleApp.Core.slnx --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "dotnet restore ./ConsoleApp/ConsoleApp.Core.slnx failed!" }
	$p = Start-Process "dotnet" -Args "restore ./Apizr/Sample.csproj --nologo -v q" -NoNewWindow -PassThru
	$p | Wait-Process
	if ($p.ExitCode -ne 0) { throw "dotnet restore ./Apizr/Sample.csproj failed!" }

[Copilot]

The dotnet restore commands at lines 166-168 do not call :throw_on_native_failure to check for errors. If a restore fails, the subsequent --no-restore build will fail with a cryptic error. For consistency with other commands in the script (e.g., line 152 which checks after publish), error checking should be added after each restore.

Suggested change

	dotnet restore .\ConsoleApp\ConsoleApp.slnx --nologo -v q
	dotnet restore .\ConsoleApp\ConsoleApp.Core.slnx --nologo -v q
	dotnet restore .\Apizr\Sample.csproj --nologo -v q
	dotnet restore .\ConsoleApp\ConsoleApp.slnx --nologo -v q
	call :throw_on_native_failure
	dotnet restore .\ConsoleApp\ConsoleApp.Core.slnx --nologo -v q
	call :throw_on_native_failure
	dotnet restore .\Apizr\Sample.csproj --nologo -v q
	call :throw_on_native_failure

[Copilot]

The BuildDockerPrefix function runs the unpinned Docker image christianhelle/refitter for smoke tests, which introduces a supply-chain risk: if that external image is ever compromised or replaced, an attacker could execute arbitrary code with access to your repository contents and any secrets available in the test environment. To reduce this risk, pin the image to an immutable digest or a trusted versioned tag under your control, and consider limiting or isolating secrets for environments where these tests run.

[Copilot]

This docker run invocation uses the external image christianhelle/refitter without a fixed tag or digest, so any upstream compromise or retagging could inject arbitrary code into your smoke-test process with access to the mounted working directory. Pin this image to a specific immutable digest or a trusted, versioned tag you control, and apply the same hardening to all similar Docker calls in this script.

[Copilot]

This settings-based generation path also runs the unpinned external Docker image christianhelle/refitter, which means a malicious or retagged image could execute arbitrary code during smoke tests with access to the checked-out source tree. Pin the Docker image reference here to an immutable digest or controlled version tag, and align it with whatever trusted source you adopt for the other Docker invocations.

[christianhelle]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@christianhelle I've opened a new pull request, #918, to work on those changes. Once the pull request is ready, I'll request review from you.