Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: run tests on windows workers #2449

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

chore: run tests on windows workers #2449

wants to merge 57 commits into from

Conversation

mdelapenya
Copy link
Collaborator

@mdelapenya mdelapenya commented Mar 26, 2024

What does this PR do?

In order to make testcontainers-go works on Windows, bringing Windows workers to the CI, this PR does the following:

  • standardises the docker image for bash in tests, from latest to a pinned version (5.2.26)
  • sets the gitatributes to avoid the CRLF conversion when cloning the repository in Windows
  • cleans up config test cases simplifying them, as they were testing too much
  • defers the closing of the Docker client in some tests, to avoid a leak
  • leverages the container lifecycle to create files in the CopyFile tests
  • removes the context with timeout in the CopyFile tests, as it was not needed
  • skips tests using host network mode, as it's not supported in Windows
  • enhances the test for reusing container in subprocess, as it was not using the right Docker socket
  • cleans up the way the default docker socket and docker socket scheme is calculated when the library is bootstrapped
  • removes an internal constant that was adding complexity. Instead we add two other variables to make the code more readable
  • extracts the checks for the Docker socket to a helper function that is testable. This allows to test the logic for the default Docker socket.It will use the Docker client infrastructure to get the correct path:
    • If the Docker client is running in a local Docker host, it will return "/var/run/docker.sock" on Unix, or "//./pipe/docker_engine" on Windows.
    • If the Docker client is running in Docker Desktop, it will return "/var/run/docker.sock" on Unix, or "//./pipe/docker_engine" on Windows.
    • If the Docker client is running in a remote Docker host, it will return "/var/run/docker.sock" on Unix, or "//var/run/docker.sock" on Windows.
    • If the Docker client is running in a rootless Docker, it will return the proper path depending on the rootless Docker configuration. Not available for windows.
    • If the Docker info cannot be retrieved, the program will panic.
  • extends the mock for the Docker client to include the OSType and infoErr fields. They will allow reproduce the different scenarios in the tests to cover the calculation of the default Docker socket
  • adds unit tests for the default Docker socket calculation, leveraging the above mock
  • skips tests that are flaky in Windows
  • do not default to the bridge driver when creating a network, instead letting the docker client decide
  • runs the core library tests (not the modules) for pull requests only on ubuntu, and on all platforms (Windows, MacOS and Ubuntu) for merge commits on main.
  • uses Testcontainers Cloud GH action for running the core library tests (not the modules) on Windows and MacOS, but only for merge commits on main.
  • it updates the Docker auth tests to create a default config.json file if it does not exist. We discovered that GH actions MacOS workers don't have that file created.

Why is it important?

Run tests on more platforms, verifying the library works on other OSs. It also "dogfoods" the usage of Testcontainers Cloud for the non-linux tests on CI.

Related issues

* main:
  feat: add module to support InfluxDB v1.x (#1703)
  feat: authenticate docker on PullImage (#2446)
  feat: add distribution-registry module (#2341)
  chore(deps): Bumping ChromaGo client version (#2402)
  chore(deps): bump github.com/docker/docker from 25.0.3+incompatible to 25.0.5+incompatible (#2444)
  feat: support passing io.Reader as ContainerFile (#2401)
a Please enter the commit message for your changes. Lines starting
@mdelapenya mdelapenya requested a review from a team as a code owner March 26, 2024 09:44
Copy link

netlify bot commented Mar 26, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 6e468bc
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66044f3c810f0e0008d45dd5
😎 Deploy Preview https://deploy-preview-2449--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

checkDefaultDockerSocket(context.Background(), mockCli{infoErr: errors.New("info should panic")}, "")
})

t.Run("Local Docker on Unix", func(tt *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiview could you double check that these expectations are correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant