Don't explicitly fail the tests during init if the screen is not loaded#8
Don't explicitly fail the tests during init if the screen is not loaded#8
init if the screen is not loaded#8Conversation
mokagio
force-pushed
the
remove-test-failure
branch
from
1e0f3cc to
9e86072
Compare
This removes a layer of abstraction. I haven't inlined the entire function yet just because I can see us using that flexible wait method in the future.
mokagio
force-pushed
the
remove-test-failure
branch
from
9e86072 to
650e4a7
Compare
| XCTAssert(result, "Screen \(self) is not loaded.") | ||
| try XCTContext.runActivity(named: "Confirm screen \(self) is loaded") { (activity) in | ||
| let result = waitFor( | ||
| element: expectedElement, | ||
| predicate: "isEnabled == true", | ||
| timeout: self.waitTimeout | ||
| ) | ||
|
|
||
| guard result == .completed else { throw WaitForScreenError.timedOut } |
There was a problem hiding this comment.
Instead of failing the tests in here, simply throw an error.
This allows consumers to catch the error if needed to continue the test execution.
| private func waitFor( | ||
| element: XCUIElement, | ||
| predicate: String, | ||
| timeout: TimeInterval | ||
| ) -> XCTWaiter.Result { | ||
| XCTWaiter.wait( | ||
| for: [ | ||
| XCTNSPredicateExpectation( | ||
| predicate: NSPredicate(format: predicate), | ||
| object: element | ||
| ) | ||
| ], | ||
| timeout: timeout | ||
| ) | ||
| } |
There was a problem hiding this comment.
This is just a minor line length and style change.
I decided to return the XCTWaiter wait(for:, timeout:) XCTWaiter.Result value instead of doing the check for it to be == .completed to make the consumer code clearer.
I didn't just inline this code in the consumer function because I can see us using this more in the future.
There was a problem hiding this comment.
I decided to return the XCTWaiter wait(for:, timeout:) XCTWaiter.Result value instead of doing the check for it to be == .completed to make the consumer code clearer.
This is a nice change!
pachlava
left a comment
pachlava
left a comment
There was a problem hiding this comment.
It was a pleasure to review this PR, thank you for doing this, Gio!
Tested by running two tests included into package - they pass.
Since the PR has no auto-merge enabled, I hope my approval will not spoil the workflow as happened with #6 which should be merged first.
| private func waitFor( | ||
| element: XCUIElement, | ||
| predicate: String, | ||
| timeout: TimeInterval | ||
| ) -> XCTWaiter.Result { | ||
| XCTWaiter.wait( | ||
| for: [ | ||
| XCTNSPredicateExpectation( | ||
| predicate: NSPredicate(format: predicate), | ||
| object: element | ||
| ) | ||
| ], | ||
| timeout: timeout | ||
| ) | ||
| } |
There was a problem hiding this comment.
I decided to return the XCTWaiter wait(for:, timeout:) XCTWaiter.Result value instead of doing the check for it to be == .completed to make the consumer code clearer.
This is a nice change!
|
Thanks @pachlava 🙌 |
2 participants
Because this logic now lives in a library, we cannot ruthlessly fail the tests anymore and need instead of be more flexible, delegating to the consumer the responsibility to decide whether a test should fail because the
ScreenObjectinstance is not yet loaded.I based this against
add/buildkitebecause I wanted CI to run on it, but it should be merged only after #6 that one goes intotrunk: