Disable/fix docker tests failing after migration to GH Actions #12625
Conversation
LocalStack Community integration with Pro2 files 2 suites 31s ⏱️ Results for commit 7100455. ♻️ This comment has been updated with latest results. |
| if isinstance(docker_client, SdkDockerClient): | ||
| pytest.skip("Test skipped for SdkDockerClient") |
There was a problem hiding this comment.
q: Do we already know how this fix will look like and when it will be delivered?
There was a problem hiding this comment.
I don’t have many details. When I asked @dfangl , he said the SDK client and the command-line client behave a bit differently, even though the API response should be the same. He plans to look into it when he has time
There was a problem hiding this comment.
The python sdk sadly does not really allow us to send the exact same request as the docker cli would - so there is some difference in the output as well, and that difference might even change between API versions of docker.
A fix would be finding out the exact difference between the API requests of the CLI and the SDK, and then patching the sdk and filing an issue upstream.
| if isinstance(docker_client, SdkDockerClient): | ||
| pytest.skip("Test skipped for SdkDockerClient") |
There was a problem hiding this comment.
The python sdk sadly does not really allow us to send the exact same request as the docker cli would - so there is some difference in the output as well, and that difference might even change between API versions of docker.
A fix would be finding out the exact difference between the API requests of the CLI and the SDK, and then patching the sdk and filing an issue upstream.
Motivation
This PR is part of initiative to migrate from CircleCI to GH Actions. After migrating the full test run from CircleCI to GitHub Actions, some tests in the
TestHooksand in theTestDockerClientclass with parameterSdkDockerClientstarted failing.Changes
test_is_container_runningwas failing because of race condition. This was fixed by adding retriestest_create_with_exposed_portswas failing forSdkDockerClient, skipping this test case for now. @dfangl will fix this test latertest_prepare_host_hook_called_with_correct_dirs- test was failing because the environment variableLOCALSTACK_INFRA_PROCESSwas set to true, likely by another test. Since environment variables are part of the global state, this caused unexpected side effect. However, the value ofLOCALSTACK_INFRA_PROCESSdoesn’t seem relevant for this particular test, and we don't expect it to be set. To avoid this issue, I modified the code to returnNoneinstead of readingos.getenv("LOCALSTACK_INFRA_PROCESS")and removed the related assertion.Testing
Tested here