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

Update run-tests.md #16710

Closed
wants to merge 1 commit into from
Closed

Update run-tests.md #16710

wants to merge 1 commit into from

Conversation

nbarnwell
Copy link

The tests literally didn't work for me. I'm not sure what breaking changes appear to have come in since they were first written, but a major problem was that calling .WithNetwork() passing in a not-yet-started network would throw an exception. Most of the changes are around coping with that - moving code from ctor to the InitializeAsync() method.

Proposed changes

Related issues (optional)

The tests literally didn't work for me. I'm not sure what breaking changes appear to have come in since they were first written, but a major problem was that calling `.WithNetwork()` passing in a not-yet-started network would throw an exception. Most of the changes are around coping with that - moving code from ctor to the `InitializeAsync()` method.
@netlify
Copy link

netlify bot commented Feb 10, 2023

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit 8275c6f
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/63e627f77dadd700080f385e
😎 Deploy Preview https://deploy-preview-16710--docsdocker.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 settings.

@craig-osterhout craig-osterhout added the area/get-started Relates to get started and onboarding docs label Feb 10, 2023
@craig-osterhout
Copy link
Contributor

Thank you, @nbarnwell. Your changes fix the exception for me.

Hi @HofmeisterAn, PTAL if you can.

@HofmeisterAn
Copy link
Contributor

HofmeisterAn commented Feb 24, 2023

I believe the appropriate solution is to set the version to 2.3.0 by using the command dotnet add package Testcontainers --version 2.3.0. By doing this, we can avoid running the documentation against an incompatible version (it looks like I did a mistake in the backwards compatibility implementation - sorry - casting the network to INetwork will work as well).

Additionally, we are in the process of implementing module support, which will make the documentation and usage much more straightforward in the future. As part of my responsibilities, I will be updating the Docker documentation once the new version is released.

It should only be necessary to update this line:

$ dotnet add package Testcontainers

Should I create a PR?

@nbarnwell
Copy link
Author

I mean, fair enough. But you are opening yourself up to having to revisit this page one day because folks will come here looking for how to use the latest version, only to find the docs stuck at v2.3. I guess choose your pain. :)

@HofmeisterAn
Copy link
Contributor

With the next release, the API will be stable. Unfortunately, despite taking great care to avoid introducing breaking changes, I was not completely successful.

But you are opening yourself up to having to revisit this page one day because folks will come here looking for how to use the latest version, only to find the docs stuck at v2.3.

It is in my own interest to ensure that the Docker documentation is up-to-date. With new TC releases, I am happy to update the documentation as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/get-started Relates to get started and onboarding docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants