Skip to content

feat: migrate from async-std to tokio #3449

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

Merged
merged 6 commits into from
Jun 27, 2022
Merged

feat: migrate from async-std to tokio #3449

merged 6 commits into from
Jun 27, 2022

Conversation

dignifiedquire
Copy link
Collaborator

@dignifiedquire dignifiedquire commented Jun 22, 2022

Working on this, to see impact and stability, as well as make it easier to integrate certain other pieces down the line

depends on

@link2xt
Copy link
Collaborator

link2xt commented Jun 22, 2022

Remove "teset" (sqlite file)

@dignifiedquire dignifiedquire marked this pull request as ready for review June 23, 2022 20:17
@@ -250,6 +250,17 @@ impl TestContext {
Self::builder().configure_fiona().build().await
}

/// Print current chat state.
pub async fn print_chats(&self) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this ever called? Previously it was in the Drop implementation for TestContext.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The nested block_on calls are not possible in tokio anymore (it has a check to avoid them and panics). As this seems to be for debugging, I figured I add this method here, and allow devs to manually add it if they want this information when debugging.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok!
cc @flub @Hocuri

@dignifiedquire dignifiedquire changed the title WIP: feat: migrate from async-std to tokio feat: migrate from async-std to tokio Jun 24, 2022
async-channel = "1.6.1"
futures-lite = "1.12.0"
tokio-stream = { version = "0.1.9", features = ["fs"] }
reqwest = { version = "0.11.11", features = ["json"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note: reqwest supports SOCKS5 so we can have SOCKS5 support for HTTPS requests now. cc @Jikstra

Copy link
Collaborator

@link2xt link2xt left a comment

Choose a reason for hiding this comment

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

Haven't tested, only looked through the changes

@link2xt
Copy link
Collaborator

link2xt commented Jun 24, 2022

cargo test fails with thread 'securejoin::tests::test_secure_join' has overflowed its stack

@dignifiedquire
Copy link
Collaborator Author

cargo test fails with thread 'securejoin::tests::test_secure_join' has overflowed its stack

yes been debugging it for a while, stack usage is just high, if I increase the stack size to 2.5Mib (instead of the default 2Mib) the test passes -.-

@link2xt
Copy link
Collaborator

link2xt commented Jun 24, 2022

This only happens in one test, maybe refactor it instead? Otherwise cargo test doesn't work locally without same env variables.

@link2xt
Copy link
Collaborator

link2xt commented Jun 24, 2022

Also test_recode_image_3 fails.

@link2xt
Copy link
Collaborator

link2xt commented Jun 24, 2022

One of the issues about high tokio stack usage in debug builds: tokio-rs/tokio#2055

Maybe instead of increasing stack size we tune our debug build to be more like release?

@dignifiedquire
Copy link
Collaborator Author

Maybe instead of increasing stack size we tune our debug build to be more like release?

did this, this fixes it more reasonable, haven't found a good way to refactor the test to fix the issue

@dignifiedquire dignifiedquire merged commit 290ee20 into master Jun 27, 2022
@dignifiedquire dignifiedquire deleted the tokio branch June 27, 2022 12:05
Hocuri added a commit that referenced this pull request Jan 9, 2023
E.g.
```
========== Chats of bob: ==========
Single#Chat#10: [email protected] [[email protected]]
--------------------------------------------------------------------------------
Msg#10:  (Contact#Contact#10): hellooo [FRESH]
Msg#11:  (Contact#Contact#10): hellooo without mailing list [FRESH]
--------------------------------------------------------------------------------

========== Chats of alice: ==========
Single#Chat#10: [email protected] [[email protected]]
--------------------------------------------------------------------------------
Msg#10: Me (Contact#Contact#Self): hellooo  √
Msg#11: Me (Contact#Contact#Self): hellooo without mailing list  √
--------------------------------------------------------------------------------
```

I found this very useful sometimes, so, let's try to re-introduce it (it
was removed in #3449)
Hocuri added a commit that referenced this pull request Jan 9, 2023
* Print chats after a test failed again

E.g.
```
========== Chats of bob: ==========
Single#Chat#10: [email protected] [[email protected]]
--------------------------------------------------------------------------------
Msg#10:  (Contact#Contact#10): hellooo [FRESH]
Msg#11:  (Contact#Contact#10): hellooo without mailing list [FRESH]
--------------------------------------------------------------------------------

========== Chats of alice: ==========
Single#Chat#10: [email protected] [[email protected]]
--------------------------------------------------------------------------------
Msg#10: Me (Contact#Contact#Self): hellooo  √
Msg#11: Me (Contact#Contact#Self): hellooo without mailing list  √
--------------------------------------------------------------------------------
```

I found this very useful sometimes, so, let's try to re-introduce it (it
was removed in #3449)

* Add failing test for TestContext::drop()

* Do not panic in TestContext::drop() if runtime is dropped

Co-authored-by: link2xt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants