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: Ensure that stderr is empty after executing scripts #1116

Merged

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Feb 13, 2024

What does this PR do?

This pull request add assertions to all tests that call ExecScriptAsync asserting that no error was written to stderr.

Adding these assertions found two issues, in the MySQL and MongoDB implementations.

MySQL would produce the following warning on stderr:

mysql: [Warning] Using a password on the command line interface can be insecure.

This has been addressed in 925f426 by writing the configuration into /etc/mysql/my.cnf instead of passing it on the command line.

MongoDB would produce the following error on stderr (when using the mongo:4.4 image in the MongoDbV4Configuration test):

/bin/sh: 1: mongosh: not found

This has been addressed in 3ceeb4b by verifying if the mongosh binary exists and choosing the proper binary instead of running mongosh || mongo through /bin/sh. This also simplified the mongosh command implementation.

Finally, I also fixed some typos: // When should be // Then in the assertion part of the tests.

Why is it important?

Having stderr empty after successfully executing a script ensures that stderr will only contain actual errors and not spurious warnings or errors that are implementation detail errors that don't actually matter.

Copy link

netlify bot commented Feb 13, 2024

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 4955a4a
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/65f3417091b3610008b7d062
😎 Deploy Preview https://deploy-preview-1116--testcontainers-dotnet.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.

@0xced 0xced force-pushed the feature/assert-sdterr-empty branch 2 times, most recently from fdf380f to a23cebb Compare February 15, 2024 22:57
@0xced 0xced force-pushed the feature/assert-sdterr-empty branch from a23cebb to 0fcffb1 Compare February 17, 2024 09:10
@HofmeisterAn HofmeisterAn changed the title Always ensure that stderr is empty after executing scripts chore: Ensure that stderr is empty after executing scripts Mar 14, 2024
@HofmeisterAn HofmeisterAn added the chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups label Mar 14, 2024
@HofmeisterAn HofmeisterAn merged commit 9156593 into testcontainers:develop Mar 14, 2024
11 checks passed
@HofmeisterAn
Copy link
Collaborator

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants