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: test cleanups #2608

Merged
merged 4 commits into from
Jun 28, 2024
Merged

chore: test cleanups #2608

merged 4 commits into from
Jun 28, 2024

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Jun 27, 2024

What does this PR do?

I was debugging failures in CI and found a few issues in code. See individual commits. I think the failure was not a real log reading failure - the container just failed to start for an unexpected reason but the test didn't catch it because it didn't check the error message.

Then, panic is a red herring too because the returned container is nil.

Why is it important?

We want clear test signals that help pinpoint the bugs, we don't want hard to understand failures.

Related issues

@ash2k ash2k requested a review from a team as a code owner June 27, 2024 03:14
Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 9d6ff02
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/667cd99ddac08f00081f4e17
😎 Deploy Preview https://deploy-preview-2608--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.

@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Jun 28, 2024
@mdelapenya mdelapenya self-assigned this Jun 28, 2024
@mdelapenya mdelapenya changed the title Cleanups chore: test cleanups Jun 28, 2024
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdelapenya
Copy link
Member

The tests failing for the rootless mode are not related. Merging

@mdelapenya mdelapenya merged commit 865a70e into testcontainers:main Jun 28, 2024
108 of 110 checks passed
mdelapenya added a commit to thaJeztah/testcontainers-go that referenced this pull request Jun 28, 2024
* main:
  chore: test cleanups (testcontainers#2608)
  chore(ci): pass docker install type to the nightly build payload (testcontainers#2612)
  chore: run rootless mode in nighlty builds (testcontainers#2611)
  chore(deps): bump github.com/hashicorp/go-retryablehttp (testcontainers#2605)
  chore: improve log handling when container is stopping (testcontainers#2601)
@ash2k ash2k deleted the cleanups branch June 28, 2024 10:59
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Jul 2, 2024
* main:
  chore(deps): bump github.com/docker/docker from v26.1.4 to v27.0.2 (testcontainers#2593)
  fix: Rename TC_HOST environment variable to TESTCONTAINERS_HOST_OVERRIDE (testcontainers#2536)
  chore: test cleanups (testcontainers#2608)
  chore(ci): pass docker install type to the nightly build payload (testcontainers#2612)
  chore: run rootless mode in nighlty builds (testcontainers#2611)
  chore(deps): bump github.com/hashicorp/go-retryablehttp (testcontainers#2605)
  chore: improve log handling when container is stopping (testcontainers#2601)
mdelapenya added a commit that referenced this pull request Jul 2, 2024
* main:
  chore: test cleanups (#2608)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Changes that do not impact the existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants