feat: add integration tests for Docker client and update CI workflow #62

Matios102 · 2025-12-09T13:15:58Z

This pull request improves the testing and coverage reporting workflow by separating unit and integration tests, ensuring Docker is available for integration tests, and merging coverage reports for comprehensive coverage analysis. It also introduces a script to run integration tests in a platform-aware manner.

Testing and coverage workflow improvements:

The GitHub Actions workflow in .github/workflows/publish-coverage.yaml now starts a Docker service (docker:dind) for integration tests, ensuring Docker is available during CI runs.
Unit and integration tests are run separately, each generating its own coverage report (coverage-unit.txt and coverage-integration.txt). These reports are then merged using gocovmerge for unified coverage reporting.
The Codecov upload step now explicitly uploads the merged coverage report and fails the CI if the upload encounters an error, improving reliability of coverage tracking.

Integration test automation:

Added a new script run_integration_tests.sh that detects the platform (macOS, Linux, Windows), configures the DOCKER_HOST environment variable as needed, checks Docker daemon accessibility, and runs integration tests with appropriate flags.

Copilot

Pull request overview

This PR introduces integration tests for the Docker client and updates the CI workflow to run both unit and integration tests separately with merged coverage reporting. The changes improve test coverage for Docker operations and ensure that Docker is available during CI runs.

Key Changes:

Added comprehensive integration tests for Docker client operations including container lifecycle, image management, and volume operations
Created a platform-aware shell script to facilitate running integration tests locally
Updated GitHub Actions workflow to run unit and integration tests separately, then merge coverage reports using gocovmerge

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
`internal/docker/docker_client_integration_test.go`	Adds 14 integration tests covering Docker client functionality including container creation, image pulling, volume mounting, and container lifecycle management
`run_integration_tests.sh`	Provides a platform-aware script that detects OS (macOS, Linux, Windows), configures Docker host, verifies Docker daemon accessibility, and runs integration tests
`.github/workflows/publish-coverage.yaml`	Updates CI workflow to start Docker service, run unit and integration tests separately, merge coverage reports, and upload to Codecov with error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/docker/docker_client_integration_test.go

.github/workflows/publish-coverage.yaml

run_integration_tests.sh

internal/docker/docker_client_integration_test.go

run_integration_tests.sh

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 16 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T13:37:27Z

run_integration_tests.sh

+    echo "Running: go test $TEST_FLAGS $suite"
+    echo ""
+
+    if ! go test $TEST_FLAGS "$suite"; then


The TEST_FLAGS variable is used unquoted in the command, which will cause word splitting issues. While this may work currently, it's a shell scripting anti-pattern that can lead to unexpected behavior if flags contain spaces. Consider using an array instead:

TEST_FLAGS=(-v -tags=integration -timeout 5m) # Then use: go test "${TEST_FLAGS[@]}" "$suite"

Or if keeping as a string, quote it properly in the command.

Copilot · 2025-12-09T13:37:28Z

internal/docker/docker_client_integration_test.go

+	// Setup test languages on first test run
+	if len(testLanguages) == 0 {
+		setupTestLanguages()
+	}


The check if len(testLanguages) == 0 and call to setupTestLanguages() is repeated in multiple test functions (here and in TestEnsureImage_AllSupportedLanguages). This logic should be consolidated to avoid duplication. Consider using sync.Once with a package-level var once sync.Once to ensure setup happens exactly once, or move this to TestMain.

Copilot · 2025-12-09T13:37:28Z

internal/docker/docker_client_integration_test.go

+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	err = dc.EnsureImage(ctx, testImageName)


This test uses testImageName without ensuring it's initialized. Add initialization check at the beginning of the test.

Copilot · 2025-12-09T13:37:28Z

.github/workflows/publish-coverage.yaml

+        run: go test -tags=integration -coverprofile=coverage-integration.txt -covermode=atomic ./...
+
+      - name: Merge coverage reports
+        run: |


The merge coverage reports step could fail silently if one of the coverage files doesn't exist. Consider adding error checking to ensure both coverage files were generated before attempting to merge them:

- name: Merge coverage reports run: | if [ ! -f coverage-unit.txt ] || [ ! -f coverage-integration.txt ]; then echo "Error: One or more coverage files are missing" exit 1 fi go install github.com/wadey/gocovmerge@latest gocovmerge coverage-unit.txt coverage-integration.txt > coverage.txt

Suggested change

run: |

run: |

if [ ! -f coverage-unit.txt ] || [ ! -f coverage-integration.txt ]; then

echo "Error: One or more coverage files are missing"

exit 1

fi

Copilot · 2025-12-09T13:37:29Z

.github/workflows/publish-coverage.yaml

-      - name: Run tests
-        run: go test -coverprofile=coverage.txt ./...
+      - name: Run unit tests with coverage
+        run: go test -coverprofile=coverage-unit.txt -covermode=atomic ./...


The unit tests will also execute integration tests because the -tags=integration build tag only controls which files are compiled, not which tests run. Running go test ./... without excluding the integration tag will compile and run ALL tests including integration tests (since files without build tags are always compiled). This means:

Integration tests will run twice (once in line 39, once in line 44)

The "unit" coverage report will actually include integration test coverage

To fix this, you should either:

Skip integration tests in the unit test step using: go test -short -coverprofile=coverage-unit.txt -covermode=atomic ./... (and ensure all integration tests check testing.Short())

Or exclude integration test files explicitly from the unit test run

Suggested change

run: go test -coverprofile=coverage-unit.txt -covermode=atomic ./...

run: go test -short -coverprofile=coverage-unit.txt -covermode=atomic ./...

Copilot · 2025-12-09T13:37:31Z

.github/workflows/publish-coverage.yaml

+        image: docker:dind
+        options: >-
+          --privileged
+          --health-cmd="docker ps"


The health check command docker ps will not work correctly with the Docker-in-Docker setup as configured. Inside the docker:dind container, the docker CLI client may not be available or properly configured to connect to the daemon. The default health check for docker:dind is typically dockerd-healthcheck or you can use wget -O- http://localhost:2375/_ping if exposing the Docker API. Consider using the image's built-in health check or a more appropriate command.

Suggested change

--health-cmd="docker ps"

--health-cmd="dockerd-healthcheck"

Copilot · 2025-12-09T13:37:31Z

internal/docker/docker_client_integration_test.go

+func setupTestLanguages() {
+	// Get all supported languages
+	supportedLangs := languages.GetSupportedLanguages()
+	for _, langStr := range supportedLangs {
+		if lang, err := languages.ParseLanguageType(langStr); err == nil {
+			testLanguages = append(testLanguages, lang)
+		}
+	}
+
+	// Use the first language's image for basic tests
+	if len(testLanguages) > 0 {
+		if image, err := testLanguages[0].GetDockerImage(""); err == nil {
+			testImageName = image
+		}
+	}
+}


The setupTestLanguages() function modifies global variables (testLanguages and testImageName) without any synchronization. If tests are run in parallel (Go's default behavior with go test), this could lead to race conditions. Multiple tests may call this function simultaneously, causing concurrent writes to these global slices and strings.

Consider one of these solutions:

Use sync.Once to ensure the setup only happens once

Call t.Parallel() carefully only in tests that don't mutate these globals

Move the setup to a TestMain function that runs before any tests

Copilot · 2025-12-09T13:37:31Z

internal/docker/docker_client_integration_test.go

+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	err = dc.EnsureImage(ctx, testImageName)


This test uses testImageName without ensuring it's initialized. Since Go test execution order is not guaranteed, if this test runs before TestNewDockerClient_Success, testImageName will be empty and cause the test to fail. Add initialization check similar to lines 59-62.

Copilot · 2025-12-09T13:37:31Z

internal/docker/docker_client_integration_test.go

+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	err = dc.EnsureImage(ctx, testImageName)


This test uses testImageName without ensuring it's initialized. Add initialization check at the beginning of the test.

Copilot · 2025-12-09T13:37:32Z

internal/docker/docker_client_integration_test.go

+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	err = dc.EnsureImage(ctx, testImageName)


This test uses testImageName without ensuring it's initialized. Add initialization check at the beginning of the test.

…on tests

feat: add integration tests for Docker client and update CI workflow

967c20e

Matios102 requested a review from a team as a code owner December 9, 2025 13:15

Matios102 requested a review from Copilot December 9, 2025 13:16

Copilot started reviewing on behalf of Matios102 December 9, 2025 13:16 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

fix: add build constraint for integration tests in docker client

d13cb1c

Matios102 requested a review from Copilot December 9, 2025 13:30

Copilot started reviewing on behalf of Matios102 December 9, 2025 13:30 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

Matios102 marked this pull request as draft December 11, 2025 15:10

feat: ensure test languages are initialized exactly once in integrati…

047f27e

…on tests

-        run: |
+        run: |
+          if [ ! -f coverage-unit.txt ] || [ ! -f coverage-integration.txt ]; then
+            echo "Error: One or more coverage files are missing"
+            exit 1
+          fi

	run: go test -coverprofile=coverage-unit.txt -covermode=atomic ./...
	run: go test -short -coverprofile=coverage-unit.txt -covermode=atomic ./...

feat: add integration tests for Docker client and update CI workflow #62

Are you sure you want to change the base?

feat: add integration tests for Docker client and update CI workflow #62

Uh oh!

Conversation

Matios102 commented Dec 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants