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.
Behavior validation docs #408
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
Behavior validation docs #408
Changes from 19 commits
df277e7
00f66c6
8a91324
cede7da
d3f3cca
8192e0f
a2bf9c0
a0dfc5b
9c1e5dd
8027b83
864fc9b
1876270
1cb63d0
03aee83
a609530
59861d7
e32836f
1074cbc
1811c9f
a5134ac
e594612
a9a17b5
fa9c4c2
9f5d89a
9eae71e
9fe5b3f
a53d724
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because
#expect(await ...)
does not expand its arguments, please split this line up:The existing code will compile, but won't produce the best possible output on test failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a
Toppings
type and aPizzaToppings
type. But this type also describes the base. Perhaps this should just bePizza
and the other types should be nestedPizza.Topping
andPizza.Base
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awkward phrasing. What are you trying to do here? We can probably simplify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to test that the range is within the domain of indices in the
pizzas
array, but given that you go on below to say that I don't need to show the code under test, I'll avoid fixing this (which isn't a complete check anyway).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of code here that isn't actually going to be used by developers. Are we sure we need to actually build out this type for the sake of explaining the topic? Is there some way to factor some of it out of the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I'll remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should emphasize here that a test function that throws will fail if an error is thrown, so usually you don't need to use
#expect(throws: Never.self)
. You'd really only use it if you want to record an issue for a thrown error but not stop execution of the test function. It's pretty nuanced, to the point that I'm not sure we should even call it out in the documentation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some history here that we don't need to document, but for your own edification/amusement: XCTest in Objective-C has
XCTAssertNoThrow()
that asserts a given expression doesn't throw an Objective-C or C++ exception. This is different from a Swift error because if an exception is thrown through a test method's frame, it tends to wreak some havoc on the way out before being caught by XCTest, so you want to proactively catch them if you know they might occur. This was inherited asXCTAssertNoThrow()
in Swift which checks if a function throws a Swift error rather than an exception.