-
Notifications
You must be signed in to change notification settings - Fork 269
Implement XCTAssertIdentical & XCTAssertNotIdentical #325
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
Conversation
|
@swift-ci please test |
| // CHECK: test.swift:[[@LINE+3]]: error: FailureMessagesTestCase.testAssertNotIdentical : XCTAssertNotIdentical failed: \("nil"\) is identical to \("nil"\) - message | ||
| // CHECK: Test Case 'FailureMessagesTestCase.testAssertNotIdentical' failed \(\d+\.\d+ seconds\) | ||
| func testAssertNotIdentical() { | ||
| XCTAssertNotIdentical(nil, nil, "message", file: "test.swift") |
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.
Thank you for this change! I think we can land it soon
My only request is that you add a few more assertions to these two tests, in particular success cases which validate that both XCTAssertIdentical and XCTAssertNotIdentical succeed when passed identical or not identical values, respectively. Currently, the tests only validate failure scenarios, but we should be sure to have tests covering when they pass as well. Personally, I'm fine just adding those inside the existing two test method bodies, to keep things simple.
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.
Oh that's a good point. I added a few succeeding assertions to the test.
3b83664 to
507bb91
Compare
|
@swift-ci please test |
| // CHECK: Test Case 'FailureMessagesTestCase.testAssertIdentical' failed \(\d+\.\d+ seconds\) | ||
| func testAssertIdentical() { | ||
| let object = XCTestExpectation() | ||
| XCTAssertIdentical(object as AnyObject, object as AnyObject) |
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.
Is it actually necessary to cast these using as AnyObject ? I would expect it to work without the casts
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.
With this specific assertion the cast wasn't needed indeed, I removed it.
All the other assertions require a cast to avoid a compiler error (type of expression is ambiguous without more context).
These functions are available in Xcode 12.5, but aren't available on Linux/Windows. This change adds the implementations for them.
507bb91 to
cdda9d7
Compare
|
@swift-ci please test |
|
Looks great. Thank you! |
Implement XCTAssertIdentical & XCTAssertNotIdentical
These functions are available in Xcode 12.5, but aren't available on Linux/Windows.
This change adds the implementations for them.