Convert JSON/PropertyList tests to swift-testing #1364
Conversation
|
@swift-ci please test |
| #expect(throws: Never.self) { | ||
| try JSONDecoder().decode(RepeatNilCheckDecodable.self, from: json) | ||
| } |
There was a problem hiding this comment.
Is the idea that this never throws? It might be more clear to mark the function as throws and then just have the code be:
| #expect(throws: Never.self) { | |
| try JSONDecoder().decode(RepeatNilCheckDecodable.self, from: json) | |
| } | |
| let _ = try JSONDecoder().decode(RepeatNilCheckDecodable.self, from: json) |
There was a problem hiding this comment.
Ah, I see that this is a specific Testing pattern. I don't totally get this distinction, but it sounds like what's there is the right code to use!
If the intent is for a test to fail when an error is thrown by expression, rather than to explicitly check that an error is not thrown by it, do not use this macro. Instead, simply call the code in question and allow it to throw an error naturally.
There was a problem hiding this comment.
Yeah my thought was that with let _ = try JSONDecoder().decode(RepeatNilCheckDecodable.self, from: json) the intent is a little ambiguous (we don't do anything with the result, so what exactly are we testing?) whereas the #expect(throws: Never.self) makes it explicitly clear that we're testing that this doesn't throw (as opposed to doesn't crash, or doesn't produce some other side effect). We do use the let _ = ... syntax elsewhere, but for cases where we're just testing things that shouldn't crash (but ideally those would have an expectation on the return value as well).
Additionally, if there is an issue, let _ = try ... stops the test immediately and does not attempt the rest of the test, whereas #expect(throws: Never.self) will continue to check other expectations even if this throws, which can get you a more complete story of failures when running in remote environments like CI.
| } | ||
| } | ||
|
|
||
|
|
||
| // MARK: - Helper Global Functions | ||
| func XCTAssertEqualPaths(_ lhs: [CodingKey], _ rhs: [CodingKey], _ prefix: String) { | ||
| func AssertEqualPaths(_ lhs: [CodingKey], _ rhs: [CodingKey], _ prefix: String) { |
There was a problem hiding this comment.
Would be nice to augment this with a sourceLocation: SourceLocation = #_sourceLocation.
There was a problem hiding this comment.
Good call! I'll update this
|
@swift-ci please test |
Continued cherry-picks (with edits) of the initial swift-testing adoption for all of our encoder/decoder-related tests
In particular, this change (in addition to adopting swift-testing):
#expect(throws:)instead ofdo/catchwithIssue.recordfor example) for older tests written before these types of expectations existedtry!/as!to help trigger expectation failures instead of crashes when unit tests failreplacingOccurrences(of:with:)andrange(of:))