8000 Get toEventually et al working in async contexts. by younata · Pull Request #1007 · Quick/Nimble · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Get toEventually et al working in async contexts. #1007

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 31, 2022
Merged

Get toEventually et al working in async contexts. #1007

merged 5 commits into from
Oct 31, 2022

Conversation

younata
Copy link
Member
@younata younata commented Oct 28, 2022

Resolves #1006

The Problem

#1006 is a result of RunLoop not being available on Swift Async contexts. I found that, effectively, these methods turn into no-ops if you run them in a swift async context (even if the function itself is not marked as async, if it's in a call hierarchy that has an async function, then that's still an async context). Which means that when the wait function continuously calls RunLoop.run(mode:before:) to spin the runloop while the matcher polling happens Await.swift:264-267, what's actually happening is that it's just continuously checking self.promise.asyncResult.isIncomplete() - almost as if the run(mode:before:) call isn't even in there. Which is a problem because using the RunLoop.run methods is how you can yield control of the current thread to other tasks.

Backing up a bit further, toEventually and waitUntil both work by combining 2 dispatch sources with control of the RunLoop. They set up one dispatch source as the timeout source and the other as the polling source (note that waitUntil does not use the polling source, but the done function argument is the equivalent of a polling source).

The timeout source uses a background queue, and runs once the timeout occurs. When the timeout happens, sets the result of the entire matcher to .timedOut and then stops the runloop (there's also logic for detecting a stalled runloop, which is unimportant to this discussion) . It's important to note that, because the timeout source is attached to a background queue, its handler is called on a background thread. This is why #1006 is "always results in a timeout error" instead of "hangs the test".

The polling source uses the main queue, and runs every pollInterval. This is where the actual matcher checking happens. This reruns the matcher, checking if it's passed. If the matcher has passed (or thrown an error), then the poller sets the result of the entire matcher to .completed and stops the runloop Awaiter.poll.

As noted earlier, waitUntil doesn't use a polling source. What it does is, when you call the done function argument, it runs logic on the main thread which sets the await result to .completed and stops the runloop Awaiter.performBlock. Which, for this bug, is effectively the same as the "continuously poll the matcher on the main thread" logic that toEventually uses. It also shows that the root of the issue is not the dispatch sources.

The dispatch sources work great in either async or synchronous contexts. As I stated earlier, the issue is that, while waiting for the matcher to get polled (or for done() to be called), AwaitPromiseBuilder.wait is refusing to yield control of the main thread.

For those curious why toEventually works when called in an async context that's not the main actor? Because in that case, wait is not running on the main thread. It's still pegging the CPU and refusing to yield control, but since it's not running on the main thread, the only consequence is unnecessarily using resources. So, it's spinning and continuously checking if the m matcher has completed/done() has been called, but not blocking the main thread.

The Solution

So, the solution? Stop using RunLoop.run() to yield control of the main thread.

Simpler said then done. The only other way I know of is to use Task.yield, but that would require me to make Wait async, and everything that calls it also async. Which is exactly what I did. I effectively duplicated the await methods, marked the old ones as noasync, and adjusted these new ones to work well in an async world.

The async version of toEventually uses a new async version of execute. The noasync version uses a Predicate under the hood so that it can re-use execute. Given that predicates aren't (yet?) Async-aware, I couldn't use that same approach. So, instead, this async version of execute is almost entirely the same as the synchronous version, but instead of taking in a predicate, it takes in a predicateExecutor (which is an async function), which returns the predicate result. This allows me to use an async version of Polling/async (this PR renames that function to poll) to call into the now async-aware versions of the old polling logic.

This was a wild bug.

@younata younata requested a review from jessesquires October 28, 2022 17:21
@younata
Get rid of noasync
d52d8f6 
- It's not available until Swift 5.7
- It was more useful during development than it will be for actual usage.
@younata
Fix all swiftlint warnings
7f03249
@younata younata changed the title To eventually in async context on main Get toEventually et al working in async contexts. Oct 31, 2022
jessesquires
jessesquires approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member
@jessesquires jessesquires left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incredible work @younata! 🎉 💯

@jessesquires
Copy link
Member

seriously, what an exemplary PR this is. 🙌🏼

thanks for the thorough explanations and documentation!

@younata younata merged commit b0b0816 into main Oct 31, 2022
@younata younata deleted the toEventually_inAsyncContext_onMain branch October 31, 2022 17:49
@younata younata mentioned this pull request Nov 2, 2022
Merged
@abushnaq-work abushnaq-work mentioned this pull request Nov 2, 2022
Closed
1 task
@ViktorSimko ViktorSimko mentioned this pull request Dec 12, 2022
Closed
1 task
renovate bot referenced this pull request in cgrindel/rules_swift_package_manager Jul 7, 2023
@renovate
chore(deps): update dependency quick/nimble to v12 (#457)
ca04ed1 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [Quick/Nimble](https://togithub.com/Quick/Nimble) | major | `from:
"10.0.0"` -> `from: "v12.0.1"` |

---

### Release Notes

<details>
<summary>Quick/Nimble (Quick/Nimble)</summary>

### [`v12.0.1`](https://togithub.com/Quick/Nimble/releases/tag/v12.0.1)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v12.0.0...v12.0.1)

#### What's Changed

- Fix wasm build by [@&#8203;ikesyo](https://togithub.com/ikesyo) in
[https://github.com/Quick/Nimble/pull/1053](https://togithub.com/Quick/Nimble/pull/1053)
- Bump cocoapods from 1.12.0 to 1.12.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1054](https://togithub.com/Quick/Nimble/pull/1054)
- Bump swiftwasm/swiftwasm-action from 5.7 to 5.8 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1057](https://togithub.com/Quick/Nimble/pull/1057)
- Make the async version of poll concurrency-safe by wrapping it in an
actor by [@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1059](https://togithub.com/Quick/Nimble/pull/1059)
- cast an empty array to avoid a warning during compile time by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1060](https://togithub.com/Quick/Nimble/pull/1060)

**Full Changelog**:
Quick/Nimble@v12.0.0...v12.0.1

### [`v12.0.0`](https://togithub.com/Quick/Nimble/releases/tag/v12.0.0)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.2.2...v12.0.0)

Nimble 12 adds the ability to using polling expectations with async
expressions. Additionally, Nimble 12 includes a number of
quality-of-life improvements and bug fixes.

#### What's Changed

- Update the README to have an accurate usage of expect by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1038](https://togithub.com/Quick/Nimble/pull/1038)
- Allow usage of toEventually with async expressions by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1039](https://togithub.com/Quick/Nimble/pull/1039)
- Replace public usage of DispatchTimeInterval with a new
NimbleTimeInterval by [@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1042](https://togithub.com/Quick/Nimble/pull/1042)
- Make NimbleTimeInterval.dispatchTimeInterval public by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1043](https://togithub.com/Quick/Nimble/pull/1043)
- Run SyncExpectation's expression in async contexts of toEventually on
the main actor. by [@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1044](https://togithub.com/Quick/Nimble/pull/1044)
- satisfyAllOf and satisfyAnyOf should only evaluate the expression
once. by [@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1045](https://togithub.com/Quick/Nimble/pull/1045)
- Rename AsyncDefaults to PollingDefaults by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1023](https://togithub.com/Quick/Nimble/pull/1023)
- Fixed Swift.package: added macCatalyst to the condition for
CwlPreconditionTesting dependency by
[@&#8203;uebelack](https://togithub.com/uebelack) in
[https://github.com/Quick/Nimble/pull/1048](https://togithub.com/Quick/Nimble/pull/1048)
- Raise minimum watchos deployment target to 7.0 by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1050](https://togithub.com/Quick/Nimble/pull/1050)
- Feature/handle multithreaded notifications by
[@&#8203;johnmckerrell](https://togithub.com/johnmckerrell) and
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1051](https://togithub.com/Quick/Nimble/pull/1051)
- Objective-C support in the Swift Package version by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1005](https://togithub.com/Quick/Nimble/pull/1005)
- Update documentation in preparation for Nimble 12 by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1052](https://togithub.com/Quick/Nimble/pull/1052)

#### New Contributors

- [@&#8203;uebelack](https://togithub.com/uebelack) made their first
contribution in
[https://github.com/Quick/Nimble/pull/1048](https://togithub.com/Quick/Nimble/pull/1048)
- [@&#8203;johnmckerrell](https://togithub.com/johnmckerrell) made their
first contribution in
[https://github.com/Quick/Nimble/pull/1051](https://togithub.com/Quick/Nimble/pull/1051)

**Full Changelog**:
Quick/Nimble@v11.2.2...v12.0.0

### [`v11.2.2`](https://togithub.com/Quick/Nimble/releases/tag/v11.2.2)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.2.1...v11.2.2)

### Highlights

Nimble v11.2.2 is a minor bug fix release which fixes the build on Xcode
14.3 and Wasm.

Thanks to [@&#8203;dymv](https://togithub.com/dymv) for fixing the Xcode
14.3 build and to [@&#8203;ikesyo](https://togithub.com/ikesyo) for
fixing the wasm build!

### Autogenerated Changelog

#### What's Changed

- Bump activesupport from 6.1.5 to 6.1.7.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1029](https://togithub.com/Quick/Nimble/pull/1029)
- Bump cocoapods from 1.11.3 to 1.12.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1032](https://togithub.com/Quick/Nimble/pull/1032)
- Fixes the build on Xcode 14.3b2 by
[@&#8203;dymv](https://togithub.com/dymv) in
[https://github.com/Quick/Nimble/pull/1033](https://togithub.com/Quick/Nimble/pull/1033)
- Bump activesupport from 7.0.4.2 to 7.0.4.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1034](https://togithub.com/Quick/Nimble/pull/1034)
- Fix wasm build by [@&#8203;ikesyo](https://togithub.com/ikesyo) in
[https://github.com/Quick/Nimble/pull/1036](https://togithub.com/Quick/Nimble/pull/1036)

#### New Contributors

- [@&#8203;dymv](https://togithub.com/dymv) made their first
contribution in
[https://github.com/Quick/Nimble/pull/1033](https://togithub.com/Quick/Nimble/pull/1033)

**Full Changelog**:
Quick/Nimble@v11.2.1...v11.2.2

### [`v11.2.1`](https://togithub.com/Quick/Nimble/releases/tag/v11.2.1)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.2.0...v11.2.1)

Fixed tripping the main thread checker in async `toEventually` checks.
Your CI should no longer report that tests erroneously crashed because
the expression's `debugDescription` dared to reference something that
needed to run on the main thread.

#### What's Changed

- Add documentation on recommended ways to configure AsyncDefaults by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1022](https://togithub.com/Quick/Nimble/pull/1022)
- Ensure that stringify'ing an expression as part of the async/await
polling infrastructure always happens on the main thread by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1024](https://togithub.com/Quick/Nimble/pull/1024)

**Full Changelog**:
Quick/Nimble@v11.2.0...v11.2.1

### [`v11.2.0`](https://togithub.com/Quick/Nimble/releases/tag/v11.2.0)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.1.1...v11.2.0)

Improved developer experience by allowing you to use the sync form of
`expect` in a test that has other usage of async test. i.e. the
following code compiles again:

```swift
class MyTest: XCTestCase {
    func testExample() {
        await someAsyncFunction()
        expect(someValue).to(equal(expectedValue))
    }
}
```

#### What's Changed

- Remove autoclosure tag with async expectations by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1020](https://togithub.com/Quick/Nimble/pull/1020)

**Full Changelog**:
Quick/Nimble@v11.1.1...v11.2.0

### [`v11.1.1`](https://togithub.com/Quick/Nimble/releases/tag/v11.1.1)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.1.0...v11.1.1)

#### What's Changed

- Fix regression where named tuples could not be compared with unnamed
tuples of the same types using the == operator by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1017](https://togithub.com/Quick/Nimble/pull/1017)
- Use uncached expression in the async versions of toEventually by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1018](https://togithub.com/Quick/Nimble/pull/1018)

**Full Changelog**:
Quick/Nimble@v11.1.0...v11.1.1

### [`v11.1.0`](https://togithub.com/Quick/Nimble/releases/tag/v11.1.0)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v11.0.0...v11.1.0)

11.1.0 **drops support** for Swift 5.6 (you must use Xcode 14 or later).
[https://github.com/Quick/Nimble/pull/1009](https://togithub.com/Quick/Nimble/pull/1009)

11.1.0 fixes a developer experience bug where you could unknowingly use
the sync version of `toEventually` in an async context, which will cause
test timeout failures.
[https://github.com/Quick/Nimble/pull/1010](https://togithub.com/Quick/Nimble/pull/1010)

That is, the following test method (XCTest-style) would compile with no
errors or warnings emitted, but fail at test runtime due to timeout
issues. In v11.1.0, this now emits a warning that you're using the wrong
version of `toEventually` (and similar).

```swift
@&#8203;MainActor func testSomething() async {
    expect(1).toEventually(equal(1)) // (in v11.0.0, this would not emit any kind of warning or error, but would definitely fail with a timeout error)
}
```

**Full Changelog**:
Quick/Nimble@v11.0.0...v11.1.0

### [`v11.0.0`](https://togithub.com/Quick/Nimble/releases/tag/v11.0.0)

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v10.0.0...v11.0.0)

This closes the [v11.0.0
milestone](https://togithub.com/Quick/Nimble/milestone/12).

### Highlights

Primarily, this release now supports running tests in async contexts.

#### Fixed

- `toEventually` et. al. now works from background threads.
[https://github.com/Quick/Nimble/pull/1000](https://togithub.com/Quick/Nimble/pull/1000)
- `toEventually` et. al. now work in async tests.
[https://github.com/Quick/Nimble/issues/1007](https://togithub.com/Quick/Nimble/issues/1007)

#### New

- Async/await support in expectation expressions (e.g. `await
expect(await someAsyncFunction()).to(...)`).
[https://github.com/Quick/Nimble/pull/1004](https://togithub.com/Quick/Nimble/pull/1004)
- `append(details:)` now respects whitespace that is in the message.
[https://github.com/Quick/Nimble/pull/1001](https://togithub.com/Quick/Nimble/pull/1001)
- watchOS support.
[https://github.com/Quick/Nimble/pull/916](https://togithub.com/Quick/Nimble/pull/916)
- You can now directly check if an expectation has passed or not.
[https://github.com/Quick/Nimble/pull/995](https://togithub.com/Quick/Nimble/pull/995)

#### Breaking

- Raised version requirements to Swift 5.6, iOS 13, macOS 10.15, tvOS
13, and watchOS 6.
[https://github.com/Quick/Nimble/issues/984](https://togithub.com/Quick/Nimble/issues/984)
- The `Expectation` struct is now a protocol. There are 2 concrete
implementations, `SyncExpectation` and `AsyncExpectation`.
`AsyncExpectation` does not support `toEventually`, and is meant for
awaiting on async functions. `SyncExpectation` is effectively the older
`Expectation` implementation, and works as it used to.
[https://github.com/Quick/Nimble/pull/1004](https://togithub.com/Quick/Nimble/pull/1004)

### Auto-generated release notes

#### What's Changed

- Add support for watchOS by
[@&#8203;JosephDuffy](https://togithub.com/JosephDuffy) in
[https://github.com/Quick/Nimble/pull/916](https://togithub.com/Quick/Nimble/pull/916)
- Add [@&#8203;younata](https://togithub.com/younata) to funding.yml by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/994](https://togithub.com/Quick/Nimble/pull/994)
- Expose whether an expectation has passed and provide an option to
throw by [@&#8203;bnickel](https://togithub.com/bnickel) in
[https://github.com/Quick/Nimble/pull/995](https://togithub.com/Quick/Nimble/pull/995)
- Raise minimum required OS and Swift Versions to Async-Support
versions. by [@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/999](https://togithub.com/Quick/Nimble/pull/999)
- Don't strip whitespace from appended newlines by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1001](https://togithub.com/Quick/Nimble/pull/1001)
- Allow toEventually to run on background threads by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1000](https://togithub.com/Quick/Nimble/pull/1000)
- Allow using async/await in expect by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1004](https://togithub.com/Quick/Nimble/pull/1004)
- Get toEventually et al working in async contexts. by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1007](https://togithub.com/Quick/Nimble/pull/1007)

#### New Contributors

- [@&#8203;JosephDuffy](https://togithub.com/JosephDuffy) made their
first contribution in
[https://github.com/Quick/Nimble/pull/916](https://togithub.com/Quick/Nimble/pull/916)
- [@&#8203;bnickel](https://togithub.com/bnickel) made their first
contribution in
[https://github.com/Quick/Nimble/pull/995](https://togithub.com/Quick/Nimble/pull/995)

**Full Changelog**:
Quick/Nimble@v10.0.0...v11.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/cgrindel/rules_swift_package_manager).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNTkuNyIsInVwZGF0ZWRJblZlciI6IjM1LjE1OS43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jessesquires jessesquires jessesquires approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

toEventually when used from an async context in the main actor always results in a timeout error.
2 participants
@younata @jessesquires
0