Segregate Docker-dependent tests to GitHub Actions workflow#49481
Segregate Docker-dependent tests to GitHub Actions workflow#49481rujche wants to merge 9 commits into
Conversation
rujche
requested review from
a team,
Netyyyy,
moarychan and
saragluna
as code owners
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent Docker/image-pull constraints in Azure DevOps CI (MCR blocked) from breaking Spring module test runs by tagging Docker-dependent tests and moving their execution to a dedicated GitHub Actions workflow that runs on a Docker-capable runner.
Changes:
- Tagged Docker-dependent JUnit 5 test classes with
@Tag("docker")in the Spring test modules. - Updated module POMs to exclude
docker-tagged tests by default and add adocker-testsMaven profile to run only those tests when desired. - Added a GitHub Actions workflow to build the Spring modules and execute the
docker-tagged tests onubuntu-24.04.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/spring/spring-cloud-azure-testcontainers/src/test/java/com/azure/spring/cloud/testcontainers/implementation/service/connection/storage/StorageQueueContainerConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Testcontainers-based queue test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-testcontainers/src/test/java/com/azure/spring/cloud/testcontainers/implementation/service/connection/storage/StorageBlobContainerConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Testcontainers-based blob test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-testcontainers/src/test/java/com/azure/spring/cloud/testcontainers/implementation/service/connection/hubs/EventHubsContainerConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Event Hubs emulator Testcontainers test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-testcontainers/src/test/java/com/azure/spring/cloud/testcontainers/implementation/service/connection/cosmos/CosmosContainerConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Cosmos emulator Testcontainers test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-testcontainers/src/test/java/com/azure/spring/cloud/testcontainers/implementation/service/connection/bus/ServiceBusContainerConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Service Bus emulator Testcontainers test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-testcontainers/pom.xml | Configures Surefire to exclude docker-tagged tests by default and introduces docker-tests profile to include them. |
| sdk/spring/spring-cloud-azure-docker-compose/src/test/java/com/azure/spring/cloud/docker/compose/implementation/service/connection/storage/StorageQueueDockerComposeConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Docker Compose-based queue test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-docker-compose/src/test/java/com/azure/spring/cloud/docker/compose/implementation/service/connection/storage/StorageBlobDockerComposeConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Docker Compose-based blob test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-docker-compose/src/test/java/com/azure/spring/cloud/docker/compose/implementation/service/connection/hubs/EventHubsDockerComposeConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Docker Compose-based Event Hubs test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-docker-compose/src/test/java/com/azure/spring/cloud/docker/compose/implementation/service/connection/bus/ServiceBusDockerComposeConnectionDetailsFactoryTests.java | Adds @Tag("docker") to mark Docker Compose-based Service Bus test for Docker-only runs. |
| sdk/spring/spring-cloud-azure-docker-compose/pom.xml | Configures Surefire to exclude docker-tagged tests by default and introduces docker-tests profile to include them. |
| .github/workflows/spring-docker-tests.yml | Adds a GitHub Actions workflow to run docker-tagged Spring tests on Ubuntu with Docker available. |
## Problem Azure DevOps network policies block Microsoft Container Registry (MCR) image pulls, causing Docker-dependent tests to fail in the default CI pipeline. ## Solution - Tag Docker-dependent tests with @tag("docker") in JUnit 5 - Configure Maven surefire to exclude docker-tagged tests from default CI - Create dedicated GitHub Actions workflow on ubuntu-24.04 with Docker support - Use docker-tests Maven profile to selectively run only tagged tests in GitHub Actions ## Changes 1. Add @tag("docker") to 9 Docker-dependent test classes: - 5 in spring-cloud-azure-testcontainers module - 4 in spring-cloud-azure-docker-compose module 2. Update pom.xml in both modules: - Configure surefire excludedGroups for docker tag - Add docker-tests Maven profile to override exclusion 3. Create .github/workflows/spring-docker-tests.yml: - Runs on ubuntu-24.04 with Docker pre-installed - Triggered by changes to spring modules and workflow file - Two-phase build: install deps, then run docker-tests profile - Triggered by pull_request and workflow_dispatch events ## Impact - Docker tests no longer block PR merges in ADO - Docker tests run independently in GitHub Actions on Linux - Default local Maven build excludes docker tests - Developers can opt-in with: mvn test -Pdocker-tests
rujche
force-pushed
the
rujche/main/update-pipeline-to-fit-network-change-for-docker-related-test
branch
from
c482169 to
500493f
Compare
| @@ -0,0 +1,54 @@ | |||
| name: Spring Docker Tests | |||
There was a problem hiding this comment.
Would this workflow be required check for pull requests?
There was a problem hiding this comment.
It's a repo level setting instead of GitHub work flow setting. Here is the detailed steps I got from Copilot:
There was a problem hiding this comment.
I knew it's branch policy settings. My question was that would you expect to set this workflow as required for PRs targeting the main branch?
There was a problem hiding this comment.
Yes. I don't have permission to configure the branch policy, could you please help to set it?
There was a problem hiding this comment.
required checks are tricky, because then they need to run on every PR and give a green no-op. maybe better to keep this optional and hope people just don't merge with it failing?
There was a problem hiding this comment.
because then they need to run on every PR
Not every PR, only PRs that modified related files.
give a green no-op
What does this mean?
|
@mikeharder could you review this PR? This is a workflow to run spring service tests. |
| pull_request: | ||
| paths: | ||
| - '.github/workflows/spring-docker-tests.yml' | ||
| - 'pom.xml' |
There was a problem hiding this comment.
Should this really trigger on every PR that changes any pom.xml?
There was a problem hiding this comment.
This is not every pom.xml, it's just the root pom.xml: https://github.com/Azure/azure-sdk-for-java/blob/main/pom.xml
…hange-for-docker-related-test
…java@v5) Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com>
Co-authored-by: Mike Harder <mharder@microsoft.com>
…hange-for-docker-related-test
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
There was a problem hiding this comment.
✅ Ready to approve
The changes consistently tag Docker-dependent tests, correctly wire Surefire filtering plus an opt-in profile, and add an appropriate GitHub Actions workflow to execute those tests on a Docker-capable runner.
Note: this review does not count toward required approvals for merging.
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
There was a problem hiding this comment.
✅ Ready to approve
The changes align with the stated CI isolation goal and only introduce minor maintainability nits around Surefire version pinning.
Note: this review does not count toward required approvals for merging.
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
There was a problem hiding this comment.
⚠️ Not ready to approve
Default builds will now run zero tests for these modules unless the new GitHub Actions workflow is reliably gating PRs (and the workflow trigger scope should be tightened to avoid unnecessary runs).
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 3
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
| pull_request: | ||
| paths: | ||
| - '.github/workflows/spring-docker-tests.yml' | ||
| - 'pom.xml' | ||
| - 'sdk/spring/**' | ||
| - 'sdk/parents/**' |
| <surefire.test.groups/> | ||
| <surefire.excluded.test.groups>docker</surefire.excluded.test.groups> |
| <surefire.test.groups/> | ||
| <surefire.excluded.test.groups>docker</surefire.excluded.test.groups> |
Description
Problem
Azure DevOps network policies block Microsoft Container Registry (MCR) image pulls, causing Docker-dependent tests to fail in the default CI pipeline.
Solution
Changes
Add @tag("docker") to 9 Docker-dependent test classes:
Update pom.xml in both modules:
Create .github/workflows/spring-docker-tests.yml:
Impact
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines