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: check error message in parts when testing the CreateContainerWithDirs method #576

Merged

Conversation

mdelapenya
Copy link
Collaborator

@mdelapenya mdelapenya commented Oct 21, 2022

What does this PR do?

In the TestDockerCreateContainerWithDirs test method, there is a test table including an error message to be checked against the possible failure modes of that feature. This error message was created as a string to be compared with the expected error message from the Docker daemon while trying to copy the dirs to the container.

In this PR, we are changing the type of that error message, from a string to an array of strings, so that we are able to check that all parts are contained in the error message from the daemon.

I acknowledge that testing an error message could lead to flakiness in the future, so we could simple remove the assertions.

Why is it important?

We have observed two different error messages in the Docker daemon for the same operation of copying a directory to a path that does not exist in the container.

Running the test on my Mac M1, where the Docker info is:

2022/10/22 00:15:50 github.com/testcontainers/testcontainers-go - Connected to docker:
    Server Version: 20.10.17
    API Version: 1.41
    Operating System: Docker Desktop
    Total Memory: 7951 MB

I get the same error message from the daemon than before these changes:

"can't copy ./testresources to container: Error: No such container:path"

But when running the test the Github actions, where we use Ubuntu:latest as build machines and the Docker info returns:

22022/10/21 22:11:26 github.com/testcontainers/testcontainers-go - Connected to docker:
    Server Version: 20.10.18+azure-
    API Version: 1.41
    Operating System: Ubuntu 20.04.5 LTS
    Total Memory: 6944 MB

we see that the error message for the very same operation is:

can't copy ./testresources to container: Error response from daemon: Could not find the file /parent-does-not-exist in container 81700aee9f0ab62c05dbbaa89798a8ab4f7843f54d7288b2a5a6360b2af19e90: failed to create container

For that reason we are adding these changes, checking the error message in parts, not the entire string.

I've done a super quick search on Github looking for changes in the daemon responses on errors, but I could not find anything yet, but I probably need more time to find the original changes. In any case, I consider the different server version relevant for the changes.

Besides that, the fix for an upcoming release will be of interest.

Finally, the test failure is blocking other PRs in a non-deterministic manner: there are times where the GHA are passing and failing, probably because we are using ubuntu:latest. I wonder if Github infra is deterministic about the builder machines 🤔

Related issues

@codecov
Copy link

codecov bot commented Oct 21, 2022

Codecov Report

Merging #576 (d76e27c) into main (be7df34) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #576   +/-   ##
=======================================
  Coverage   15.40%   15.40%           
=======================================
  Files          13       13           
  Lines        1759     1759           
=======================================
  Hits          271      271           
  Misses       1442     1442           
  Partials       46       46           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mdelapenya
Copy link
Collaborator Author

@kiview I'm merging this now as it's a refactor in the assertions of a specific test, not impacting functionality

@mdelapenya mdelapenya merged commit 9bee089 into testcontainers:main Oct 21, 2022
@mdelapenya mdelapenya deleted the consistency-in-test-messages branch October 21, 2022 23:29
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