test: Add tests for the tar writer #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
euanh
force-pushed
the
tar-testing
branch
2 times, most recently
from
eb59695 to
1196f8c
Compare
Preparation for refactoring and unit testing
Splitting the tar function makes it easier to unit test and to support multiple files in an archive later.
Squashing multiple swift-testing arguments onto the same line makes the tests very hard to maintain.
Test the underlying helper methods used to write tar headers.
euanh
force-pushed
the
tar-testing
branch
2 times, most recently
from
81a71c6 to
2cf2ab5
Compare
euanh
changed the title
String(format:) sometimes returns an empty string even if the number
being formatted can be represented in an octal string of the requested
length. This may be a thread-safety problem and has only been seen
when running the tests.
swiftlang/swift-corelibs-foundation#5152
This commit changes octal6/11 to use String(value, radix:) and
handle padding directly. This has not failed during hundreds of
test runs.
Benchmarking the old and new implementations of octal6() shows that
the new version also is about twice as fast as the old one and makes
no allocations, whereas the old version made 7 allocations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The
tarwriter is tested by the end to end tests, but unit tests are more helpful for refactoring and extending it.Modifications
tarinto its own moduletarheadersSwift(format:)to convert integers into octal strings because ofString(format:)intermittently returns an empty string when run concurrently byswift-testingswiftlang/swift-corelibs-foundation#5152Result
The basic helper functions underpinning the tar writer will have unit tests, making it easier to test, refactor and extend the tar writer in future.
Test Plan
This pull request adds new tests. All existing tests continue to pass.