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

Rewrite complexapp test documentation #5926

Merged
merged 7 commits into from
Oct 1, 2024

Conversation

lbussell
Copy link
Contributor

What I did:

  • Consolidated test documentation into run-tests-in-container.md instead of splitting it between there and complexapp/README.md
  • Added section on running tests with Docker build using BuildKit's new(-ish) build output feature
  • Re-ran example commands to get new output for .NET 9
  • Removed the strict recommendation for running tests as an executable stage over running tests as part of Docker build. With BuildKit, they seem about equal to me.

Potential for improvement:

  • I have not played with getting test output from the Docker build without buildkit. Theoretically you could build the test-output stage and then start the container and copy the test results out. However, if you don't have BuildKit you probably don't want to use this approach anyways. Since without BuildKit, the Docker builder won't skip stages in a multi-stage build.

samples/complexapp/Dockerfile Outdated Show resolved Hide resolved
samples/complexapp/tests/StringLibraryTests.cs Outdated Show resolved Hide resolved
samples/run-tests-in-sdk-container.md Outdated Show resolved Hide resolved
Copy link
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

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

Didn't mean to approve yet. Changing my status.

@lbussell lbussell requested a review from mthalman October 1, 2024 17:52
@lbussell lbussell enabled auto-merge (squash) October 1, 2024 18:25
@lbussell lbussell merged commit cc108aa into dotnet:main Oct 1, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants