Separate Terraform's tool-dependent tests into a new Calamari.ExternalTools.Tests project - #2107
Separate Terraform's tool-dependent tests into a new Calamari.ExternalTools.Tests project#2107zentron wants to merge 16 commits into
Conversation
zentron
changed the title
zentron
left a comment
zentron
left a comment
There was a problem hiding this comment.
Left a few pointers on things worth knowing before/while reviewing.
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Calamari.DockerCredentialHelper", "Calamari.DockerCredentialHelper\Calamari.DockerCredentialHelper.csproj", "{B34DBEEC-7AC2-4BFE-ACDD-1788828925BD}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Calamari.ExternalTools.Tests", "Calamari.ExternalTools.Tests\Calamari.ExternalTools.Tests.csproj", "{028A8E26-0E75-48C2-A639-2FE0D8B45DEE}" |
There was a problem hiding this comment.
This branch was rebased onto a main-line commit that separately removed Calamari.AiAgent/AiAgent.Tests (consolidated into core Calamari, #2073). The rebase's text-merge briefly resurrected the AiAgent.Tests entries as an artifact; they're cleaned up in a later commit on this branch. This file should now only differ from main by this one added entry plus unrelated indentation reflow further down.
| static void AddExecutePermission(string exePath) | ||
| { | ||
| if (CalamariEnvironment.IsRunningOnWindows || string.IsNullOrEmpty(exePath)) | ||
| return; | ||
|
|
||
| SilentProcessRunner.ExecuteCommand( | ||
| "chmod", $"+x {exePath}", | ||
| Path.GetDirectoryName(exePath) ?? ".", | ||
| _ => { }, _ => { }); | ||
| } | ||
|
|
||
| static HttpClient CreateHttpClient() | ||
| { | ||
| var client = new HttpClient(); | ||
| client.DefaultRequestHeaders.UserAgent.ParseAdd( | ||
| "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/136.0.0.0 Safari/537.36"); | ||
| return client; |
There was a problem hiding this comment.
Known gaps carried over from the old fixture, not fixed here: chmod's exit code isn't checked (a failure surfaces later as a confusing permission error), and there's no checksum verification against HashiCorp's published SHA256SUMS before extracting/executing the archive. Worth closing before more tools lean on this shared code — scoping it out of this PR to keep the diff focused.
| return null; | ||
| } | ||
|
|
||
| public static string? GetInstalledVersion(string executablePath, string versionArg = "--version") |
There was a problem hiding this comment.
Returns the raw multi-line --version banner, not a clean semver string. Currently harmless — the only consumer (CopyAllFiles's version-named-subfolder lookup) just no-ops when nothing matches — but if a future tool's tests rely on this for real branching, it'll need --version --json plus parsing.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Reimplements the custom tooling mechanism from feature/external-tool-test-separation against current main, scoped to infrastructure + Terraform. Supersedes the in-place categorisation branches as the canonical approach.
…rraform) Task-by-task TDD plan reimplementing the tool-manifest/resolver/downloader mechanism against current main, plus the Terraform migration. Adds real unit coverage to the existing Calamari.Terraform.Tests fixtures (which already have internals access) instead of duplicating logic elsewhere.
…ersion range checks
Rebasing onto main (which removed Calamari.AiAgent/.Tests as part of the Claude consolidation, #2073) reintroduced the AiAgent.Tests sln entries as a side effect of the original malformed-EndProject fix in Task 1. The project no longer exists on disk; drop the dangling entry.
Closes a final-review finding: ExternalToolFixture checked PATH unconditionally before downloading, so any tool found on PATH silently won over the manifest-pinned version regardless of CALAMARI_TOOL_SKIP_DOWNLOAD, defeating reproducibility. PATH is now only consulted when the flag is set, and downloading is the default; if the flag is set but the tool isn't on PATH, resolution now fails loudly instead of falling through to download.
Neither had unit coverage of their Execute() logic - apply arg construction, output-JSON parsing and sensitivity flagging, and the plan detailed-exit-code handling. Mocks ICommandLineRunner the same way TerraformCliExecutorFixture does, so no real terraform binary is needed.
…ests WithOutputSensitiveVariables and PlanDetailedExitCode only exercised Calamari's own output/exit-code handling, not terraform's behaviour - that's now covered by ApplyBehaviourFixture/PlanBehaviourFixture without needing a real terraform binary.
…rget Folds it into the existing publish/zip pipeline (no signing, same as other .Tests projects) so a build breakage is caught on every commit instead of only when the nightly TestCalamariExternalTools run happens to build it. That new target follows TestCalamariFlavourProject's shape - points at the pre-built CalamariTests/ dll, filtered to TestCategory=ExternalTool.
61aeda1 to
d888df6
Compare
1 participant
Background
Terraform's integration tests lived in
Calamari.Terraform.Testsand ran in the default pipeline, but most need a realterraformbinary plus network/cloud access — slow, and not reproducible (whicheverterraformwas installed got used, silently). Some ofTerraformCliExecutor/ApplyBehaviour/PlanBehaviour's CLI argument construction and output parsing also had no unit coverage.Results
Calamari.ExternalTools.Testsproject: reusable tool-manifest/resolver/downloader infrastructure,[Category("ExternalTool")].ExternalToolFixtureresolves via env var pin →CALAMARI_TOOL_SKIP_DOWNLOAD=true(PATH-only) → download the manifest's pinned version by default.Calamari.Terraform.Teststrimmed to pure-unit fixtures, stays in the default pipeline.-get-pluginsversion gate, "untested version" warning, env-var JSON parsing,ApplyBehaviour's apply-arg construction and output/sensitivity parsing,PlanBehaviour's detailed-exit-code handling — all against a mockedICommandLineRunner, no real terraform involved.TerraformCommandsFixturetrimmed further:WithOutputSensitiveVariablesandPlanDetailedExitCoderemoved now that the new mocked tests cover the same Calamari-side logic. Left: one e2e smoke test, 2 wiring tests, 3 cloud tests.Calamari.ExternalTools.Testsis now published/zipped byPackageCalamariProjects(no signing, same as other.Testsprojects) so a build breakage is caught on every commit. NewTestCalamariExternalToolsNuke target runs it — same shape asTestCalamariFlavourProject, filtered toTestCategory=ExternalTool— meant to be invoked as its own nightly/on-demand TeamCity build, not part ofBuildCi.Testing
dotnet build: clean, including the Nuke build project itself (dotnet build build/_build.csproj) and./build.sh --helpconfirms the new target is registered.Calamari.Terraform.Tests: 34/34. New infra unit tests: 12/12.TerraformCommandsFixturenon-cloud: pass; cloud ones need credentials this environment lacks; AWS is[Ignore]d (S3 ACL issue).How to review
Most of the diff is
git mv. Real logic is inInfrastructure/*.cs(5 small files), the new/updated tests inTerraformCliExecutorFixture.cs/ApplyBehaviourFixture.cs/PlanBehaviourFixture.cs, and the twobuild/*.cschanges for the nightly test target.Calamari.slnhas unrelated indentation noise fromdotnet sln add.Caveats
TestCalamariExternalToolsisn't wired into any TeamCity build config yet — that's a follow-up (needs a config that runs on a schedule and knows to fetch the newCalamari.ExternalTools.Tests.zipartifact intoCalamariTests/, mirroring howTestCalamariFlavourProjectconsumes each flavour's zip).No.