Skip to content
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

increase test coverage for async-await #62121

Closed
nikomatsakis opened this issue Jun 25, 2019 · 5 comments
Closed

increase test coverage for async-await #62121

nikomatsakis opened this issue Jun 25, 2019 · 5 comments

Comments

@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Jun 25, 2019

As part of the async-await feature we want to make sure we have good test coverage. We have a master list of tests prepared by @cramertj and @Centril that we'd like to complete.

@cramertj
Copy link
Member

@cramertj cramertj commented Jun 25, 2019

#62106 is a first step towards this.

@Centril
Copy link
Contributor

@Centril Centril commented Jun 28, 2019

Centril added a commit to Centril/rust that referenced this issue Jun 30, 2019
…etrochenkov

Always parse 'async unsafe fn' + properly ban in 2015

Parse `async unsafe fn` not `unsafe async fn` in implementations. We also take the opportunity to properly ban `async fn` in Rust 2015 when they are inside implementations.

Closes rust-lang#62232.

cc rust-lang#61319, rust-lang#62121, and rust-lang#62149.

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this issue Jul 5, 2019
…=Centril

Create async version of the dynamic-drop test

Some of the tests in dynamic-drop have been cut:
* The tests that are just simpler versions of other tests - these tests are already fairly slow due to all of the unwinding and async functions have more control flow paths than normal functions.
* The union test - it's for an unstable feature that has an RFC to remove it.
* The generator test - there aren't async generators yet.
* The tests that show values being leaked - these can be added once the issue is fixed.

r? @Centril
cc  rust-lang#62121 @cramertj
Centril added a commit to Centril/rust that referenced this issue Jul 5, 2019
…=Centril

Create async version of the dynamic-drop test

Some of the tests in dynamic-drop have been cut:
* The tests that are just simpler versions of other tests - these tests are already fairly slow due to all of the unwinding and async functions have more control flow paths than normal functions.
* The union test - it's for an unstable feature that has an RFC to remove it.
* The generator test - there aren't async generators yet.
* The tests that show values being leaked - these can be added once the issue is fixed.

r? @Centril
cc  rust-lang#62121 @cramertj
Centril added a commit to Centril/rust that referenced this issue Jul 5, 2019
…=Centril

Create async version of the dynamic-drop test

Some of the tests in dynamic-drop have been cut:
* The tests that are just simpler versions of other tests - these tests are already fairly slow due to all of the unwinding and async functions have more control flow paths than normal functions.
* The union test - it's for an unstable feature that has an RFC to remove it.
* The generator test - there aren't async generators yet.
* The tests that show values being leaked - these can be added once the issue is fixed.

r? @Centril
cc  rust-lang#62121 @cramertj
@delan
Copy link
Contributor

@delan delan commented Jul 11, 2019

as discussed in #62518, I can do some async unsafe fn tests starting with

  • Check that an async unsafe fn requires unsafe { ... } to be called.
    • Including async unsafe fn implementations...
  • Tangentially also test the behavior of unsafe { async { $stuff } } wrt. allowing unsafe operations in $stuff.

Centril added a commit to Centril/rust that referenced this issue Jul 12, 2019
Centril added a commit to Centril/rust that referenced this issue Aug 6, 2019
tests for async/await drop order

This is just me helping out with rust-lang#62121 where I can.

I'm also going to use this as a public place to collect my thoughts about what has already been done and what hasn't (adding comments to the dropbox paper doc was quickly getting spammy).

I've tried to keep my commit messages similar to the line items on https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiKouT0L41mSnK1741s~TiiRAg-nMyZGrra7dz9KcFRMLKJy as possible.

A bunch of my tests are likely to be either redundant with other tests, or lower quality than other tests that people are writing. A reasonable approach might be to tell me which commits you want to keep and I'll throw away the rest of them.

The part from the dropbox paper doc that I'm concentrating on here is:
(items marked with `?` are ones that I can't immediately think of how to test, so I will leave for other people. Items with checkboxes are things that I have done or will try to do next)

### Dynamic semantics
- `async`/`await` with unusual locals:
    - ? partially uninhabited
    - ? conditionally initialized
    - ~drop impls~ already done in src/test/ui/async-await/drop-order/*
    - ? nested drop impls
    - ~partially moved (e.g., `let x = (vec![], vec![]); drop(x.0); foo.await; drop(x.1);`)~ see  rust-lang#63310
- Control flow:
    - basic
    - complex
- [x] `.await` while holding variables of different sizes
- (possibly) drop order
    - [x] including drop order for locals when a future is dropped part-way through execution
         - Parameters' drop order is covered in my commit f40190a
    - ~An async fn version of `dynamic-drop.rs`~
        - already done by matthewjasper in https://github.com/rust-lang/rust/pull/62193/files
- ? interaction with const eval, promoteds, and temporaries
- [x] drop the resulting future and check that local variables and parameters are dropped in the expected order (interaction with cancellation, in other words)
    - also in f40190a

Explanation of commits:

* 0a1bdd4 is the simplest place I could think of to explicitly test `.await` while holding variables of different sizes. I'm pretty sure that this will end up being redundant with something else, so I'm happy to drop it.
* f40190a is a copy-paste from `drop-order-for-async-fn-parameters.rs` with `NeverReady.await` dumped on the end of each testcase.
    * Normally I don't like copy-paste-based tests, but `drop-order-for-async-fn-parameters-by-ref-binding.rs` is also copy-paste, so I thought it might be okay.
    * [x] I'm a bit sad that this doesn't cover non-parameter locals, but I think it should be easy enough to extend in that direction, so I might have a crack at that tomorrow.
* c4940e0 makes a bunch of local variables and moves them into either `{}` blocks or `async move {}` blocks, checking for any surprising differences.
    * I have tried to give the test functions descriptive names
    * I have not duplicated the tests for methods with/without self.
    * I think that all of these tests could be rewritten to be clearer if I could write down the expected drop order next to each test.
Centril added a commit to Centril/rust that referenced this issue Aug 7, 2019
Test conditional initialization validation in async fns

r? @cramertj

Per [paper doc](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiWF2Nt8tgDiA70qFI~oiLOOAg-nMyZGrra7dz9KcFRMLKJy) calling for async/.await tests, tests are desired for conditionally initialized local variables. This PR hopes to provide tests for that.

rust-lang#63294 seems to be tracking the items from the paper doc that this PR is related to
rust-lang#62121 is an open issue asking for more async/.await tests that this relates to

---
👍 executed 2 new tests
👍 tidy
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 7, 2019
Test conditional initialization validation in async fns

r? @cramertj

Per [paper doc](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiWF2Nt8tgDiA70qFI~oiLOOAg-nMyZGrra7dz9KcFRMLKJy) calling for async/.await tests, tests are desired for conditionally initialized local variables. This PR hopes to provide tests for that.

rust-lang#63294 seems to be tracking the items from the paper doc that this PR is related to
rust-lang#62121 is an open issue asking for more async/.await tests that this relates to

---
👍 executed 2 new tests
👍 tidy
Centril added a commit to Centril/rust that referenced this issue Aug 8, 2019
Test conditional initialization validation in async fns

r? @cramertj

Per [paper doc](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiWF2Nt8tgDiA70qFI~oiLOOAg-nMyZGrra7dz9KcFRMLKJy) calling for async/.await tests, tests are desired for conditionally initialized local variables. This PR hopes to provide tests for that.

rust-lang#63294 seems to be tracking the items from the paper doc that this PR is related to
rust-lang#62121 is an open issue asking for more async/.await tests that this relates to

---
👍 executed 2 new tests
👍 tidy
Centril added a commit to Centril/rust that referenced this issue Aug 8, 2019
Test conditional initialization validation in async fns

r? @cramertj

Per [paper doc](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiWF2Nt8tgDiA70qFI~oiLOOAg-nMyZGrra7dz9KcFRMLKJy) calling for async/.await tests, tests are desired for conditionally initialized local variables. This PR hopes to provide tests for that.

rust-lang#63294 seems to be tracking the items from the paper doc that this PR is related to
rust-lang#62121 is an open issue asking for more async/.await tests that this relates to

---
👍 executed 2 new tests
👍 tidy
Centril added a commit to Centril/rust that referenced this issue Aug 8, 2019
…sts, r=cramertj

Test interaction between `async { ... }` and `?`, `return`, and `break`

Per the second checkbox in rust-lang#62121 (comment), test that `async { .. }` blocks:
1. do not allow `break` expressions.
2. get targeted by `return` and not the parent function.
3. get targeted by `?` and not the parent function.

Works towards resolving blockers in rust-lang#63209.

r? @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 12, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 13, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 13, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Aug 14, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
…s, r=nikomatsakis

`async fn` lifetime elision tests

Add `async fn` version of the tests in rust-lang#61207 per the first checkbox in rust-lang#62121 (comment).
Works towards resolving blockers in rust-lang#63209.

r? @nikomatsakis
cc @cramertj
@Centril
Copy link
Contributor

@Centril Centril commented Aug 18, 2019

Thanks to everyone who contributed tests! I believe the test suite is now substantially more robust than from the outset of starting the test drive. While it is desirable to continue the drive for tests (and please do contribute more tests in areas you find lacking!), I think the testsuite is non-blocking in terms of stabilization. Therefore I'm relabeling this to -Deferred.

Centril added a commit to Centril/rust that referenced this issue Aug 20, 2019
…amertj

Stabilize `async_await` in Rust 1.39.0

Here we stabilize:
- free and inherent `async fn`s,
- the `<expr>.await` expression form,
- and the `async move? { ... }` block form.

Closes rust-lang#62149.
Closes rust-lang#50547.

All the blockers are now closed.

<details>
- [x] FCP in rust-lang#62149
- [x] rust-lang#61949; PR in rust-lang#62849.
- [x] rust-lang#62517; PR in rust-lang#63376.
- [x] rust-lang#63225; PR in rust-lang#63501
- [x] rust-lang#63388; PR in rust-lang#63499
- [x] rust-lang#63500; PR in rust-lang#63501
- [x] rust-lang#62121 (comment)
    - [x] Some tests for control flow (PR rust-lang#63387):
          - `?`
          - `return` in `async` blocks
          - `break`
    - [x] rust-lang#61775 (comment), i.e. tests for rust-lang#60944 with `async fn`s instead). PR in rust-lang#63383

</details>

r? @cramertj
@nikomatsakis
Copy link
Contributor Author

@nikomatsakis nikomatsakis commented Aug 28, 2019

This issue has served its purpose. I'm going to close. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants