Segregate Docker-dependent tests to GitHub Actions workflow#49481
Segregate Docker-dependent tests to GitHub Actions workflow#49481rujche wants to merge 18 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
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.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
rujche
force-pushed
the
rujche/main/update-pipeline-to-fit-network-change-for-docker-related-test
branch
from
8e4faf1 to
154c419
Compare
There was a problem hiding this comment.
⚠️ Not ready to approve
The new GitHub Actions workflow uses -DskipITs in a way that’s redundant/misleading for Spring modules (Failsafe skipping is typically controlled via other properties), so the workflow should be adjusted for clarity and correctness.
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.
- Remove redundant -DskipITs from docker test execution step (Maven test phase doesn't use Failsafe) - Narrow paths filter to only affected modules and workflow file to reduce unnecessary CI runs - Update GitHub Actions to current stable versions (checkout@v4, setup-java@v4)
The docker-tests workflow needs to be triggered for all spring module changes because other modules may affect dependencies and the build of the docker test modules. Narrowing to specific modules could miss important dependency changes.
Do not change actions versions - they were already correct in the original PR.
Add optimization flags to the dependency build step: - Skip checks (checkstyle, spotbugs, jacoco, javadoc, revapi, enforcer) - Skip code snippet processing - Add dev profile - Use parallel threads (-T 4) - Disable transport progress (-ntp) These flags align with the recommended build command in sdk/spring/README.md and significantly reduce build time by skipping unnecessary checks during the dependency preparation phase.
Add optimization flags to the test execution step: - Skip checks (checkstyle, spotbugs, jacoco, javadoc, enforcer) - Use parallel threads (-T 4) - Disable transport progress (-ntp) These flags speed up test execution without affecting test quality, as checks should be enforced during the build/install phase.
The test step explicitly runs docker-tagged tests with -Pdocker-tests. If no tests are found, it should fail to catch configuration issues rather than silently succeeding. This parameter is only needed in the install step where -DskipTests is used.
- Add =true to -Dcodesnippet.skip and -Denforcer.skip in install step for consistency - Add -Dcodesnippet.skip=true to test step (was missing) - Add =true to -Denforcer.skip in test step for consistency - All skip properties now use explicit =true format
There was a problem hiding this comment.
✅ Ready to approve
The changes cleanly separate Docker-dependent tests via consistent JUnit tagging, Maven filtering, and a dedicated workflow without introducing functional code-path changes.
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.
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
mvn test -Pdocker-testsAll SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines