Adds more unit tests for WCPairing #437
Conversation
|
Btw, while on |
| func testUpdateExpiry_WhenNewExpiryDateIsLessThanExpiryDate_ShouldThrowInvalidUpdateExpiryValue() { | ||
| let expiryDate = referenceDate.advanced(by: 10 * .minute) | ||
| var pairing = WCPairing(topic: "", relay: .stub(), peerMetadata: .stub(), expiryDate: expiryDate) | ||
| XCTAssertThrowsError(try pairing.updateExpiry(40 * .day)) { error in |
There was a problem hiding this comment.
The expire date you set is 10 minutes into the future, the update value should be less than this, like the test says, but you're testing with a 40-day date (greater than max, like the test above).
| pairing.activate() | ||
| XCTAssertThrowsError(try pairing.updateExpiry(10 * .minute)) { error in |
There was a problem hiding this comment.
activate() makes a call to updateExpiry() internally (which sets a 30-day expiry).
Is there a reason to call activate before a throwing update? Does it make a difference for this test to invert the order?
There was a problem hiding this comment.
I was trying to check the error inside the activate() function, but if I understood it right, the error dies inside activate(). I changed the test, now checking for the expiryDate update. If an error occurs, it shouldn't be updated.
|
lgtm, more tests is always better 👍 |
|
@flypaper0 updated! Do these new names fit better? |
While working on this I noticed that the
activatefunction sets the status totrueeven ifupdateExpirythrows an error. Is that the expected behavior? Maybeactivateshould only change theactivevar?