Separate Azure App Service external-dependency tests from unit tests#2030
Separate Azure App Service external-dependency tests from unit tests#2030zentron wants to merge 2 commits into
Conversation
Introduce a single ExternalCloudIntegration test category and apply it to every Azure test fixture that authenticates against or calls a real Azure service. Tests without the category are treated as unit/integration tests that need no external service, so CI can run them with no credentials and run the real-cloud fixtures as a separate smoke suite. The two AppService integration base classes are tagged so all derived fixtures inherit the category (NUnit categories are inherited). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zentron
changed the title
zentron
left a comment
zentron
left a comment
There was a problem hiding this comment.
Inline notes on the test moves — pointing out why things were removed, consolidated, or pushed down to unit tests, since a red diff doesn't carry that on its own.
| { | ||
| [TestFixture] | ||
| public class WhenUsingAWindowsDotNetAppService : AppServiceIntegrationTest | ||
| public class WhenDeployingToAWebAzureApp : AzureAppServiceWithProvisionedResourcesTestBase |
There was a problem hiding this comment.
Three real-cloud tests were removed from this fixture, none of which exercised distinct Calamari logic:
- WAR/JAR deploys — both route through the same
JavaPackageProvider, differing only by Kudu upload URL; that routing is now unit-tested inPackageProviderFactoryFixture. *_WithAsyncDeploymentAndPolling— behavioural duplicates. The async path runs wheneverSupportsAsynchronousDeploymentis true (zip/nuget), so the baseline tests already coverUploadZipAndPollAsync; the feature-toggle arrange was a no-op (read toggles, write them straight back).DeployToTwoTargetsInParallel_Succeeds— guarded package-file share mode (a generic Calamari staging concern, not Azure-specific) via a full cloud deploy that asserted onlyOutcome==Successful.
| namespace Calamari.AzureAppService.Tests; | ||
|
|
||
| [TestFixture] | ||
| public class PackageProviderFactoryFixture |
There was a problem hiding this comment.
Replacement coverage for the removed WAR/JAR cloud deploy tests. The extension -> packaging strategy / Kudu URL / sync-async mapping is the only Calamari-owned decision in an otherwise Azure-bound deploy, so it's unit-tested here rather than exercised through real deploys of each package type.
|
|
||
| [TestFixture] | ||
| public class WhenUsingALinuxAppService : AppServiceIntegrationTest | ||
| public class WhenUsingALinuxAzureAppService : AzureAppServiceWithProvisionedResourcesTestBase |
There was a problem hiding this comment.
Collapsed from Windows + Linux to Linux only. The Windows/Linux FxVersion branch is unit-tested both ways in AzureAppServiceContainerDeployBehaviourUnitTestFixture, so one OS is enough to confirm the real-cloud ARM round-trip. Trade-off: the Windows FxVersion PATCH is no longer hit against real Azure (covered by the unit test's Windows branch).
| namespace Calamari.AzureAppService.Tests; | ||
|
|
||
| [TestFixture] | ||
| public class AzureAppServiceContainerDeployBehaviourUnitTestFixture |
There was a problem hiding this comment.
New unit fixture covering the OS-branch, registry-settings and slot-targeting logic that previously needed four real-cloud container tests. Uses a mocked IAzureAppServiceContainerConfigurer; the cloud fixture now keeps a single Linux smoke test.
| /// ExternalCloudIntegration category, which all descendants inherit. | ||
| /// </summary> | ||
| [Category(TestCategory.ExternalCloudIntegration)] | ||
| public abstract class AzureAppServiceTestBase |
There was a problem hiding this comment.
Consolidated root base: the auth + ArmClient setup that was duplicated across the two former base classes now lives here once, with subclasses adding only their resource lifecycle (provisioned vs static). Relies on NUnit running this base OneTimeSetUp before the derived one — that ordering is the main thing worth confirming in a real-cloud CI run.
| /// which target discovery talks to Azure; mocking it lets the tag-matching and target-creation | ||
| /// logic in <see cref="Behaviors.TargetDiscoveryBehaviour" /> be tested without a real Azure connection. | ||
| /// </summary> | ||
| public interface IAzureWebAppDiscoverer |
There was a problem hiding this comment.
One of two seams introduced this PR (the other is IAzureAppServiceContainerConfigurer). Extracting the single Azure call behind an interface is what lets the surrounding tag-matching / slot / service-message logic be unit-tested with a mock instead of a live subscription.
| const string WebAppName = "calamari-testing-static-target-discovery"; | ||
|
|
||
| [Test] | ||
| public async Task DiscoverWebAppsAndSlots_ReturnsTheStaticTestWebApp() |
There was a problem hiding this comment.
What remains of the old 237-line integration fixture: a single smoke test that the real Azure Resource Graph query + auth still work. All the tag-matching/slot/service-message scenarios it used to run against Azure now live in TargetDiscoveryBehaviourUnitTestFixture.
| </None> | ||
| <None Update="functionapp.1.0.0.zip"> | ||
| <None Update="ExternalCloudIntegration/functionapp.1.0.0.zip"> | ||
| <Link>functionapp.1.0.0.zip</Link> |
There was a problem hiding this comment.
functionapp.1.0.0.zip moved into the ExternalCloudIntegration/ folder alongside the only test that uses it; the <Link> keeps it copying to the output root so the test still resolves it via the assembly directory. The sample.1.0.0.war / sample4-1.0.0.jar entries were removed — those binaries were orphaned when the WAR/JAR cloud tests were deleted.
1 participant
Background
The
Calamari.AzureAppService.Testssuite mixed fast, deterministic checks of Calamari's own logic with slow tests that authenticate against and provision real Azure. Interleaved, the whole suite effectively needed Azure credentials even for logic that doesn't.This branch separates them, working towards a simple rule: if a test doesn't need Azure it should be a unit test; if it does, it should be a thin, clearly-marked smoke test.
What changed
TestCategory.ExternalCloudIntegrationand tag the real-cloud base classes, so CI can run the credential-free suite separately from the live-Azure one (commit 1).ExternalCloudIntegration/folder so the split is visible in the tree.IAzureWebAppDiscoverer,IAzureAppServiceContainerConfigurer) so target-discovery and container-deploy logic can be unit-tested with mocks.Net: the credential-free suite grows, the real-cloud suite shrinks to genuine smoke tests.
Testing
dotnet test --filter "TestCategory!=ExternalCloudIntegration"→ 30/30.ExternalCloudIntegrationfixtures were not run this branch (no Azure creds in dev) — verified by compilation and review only. The splitOneTimeSetUpin the consolidated base classes (base authenticates, derived provisions/looks-up) wants one real CI run to confirm ordering.How to review
Two commits, in order: the first only categorises; the second does the refactor. Production change is small and mostly mechanical (the two seams + their DI registration in
Program.cs); the rest is test moves. I've left inline comments on each removal/consolidation.