refactor: update crate dependencies#3216
Conversation
Robot Results
|
rina23q
left a comment
There was a problem hiding this comment.
create() was not replaced by create_async() in two files.
| let _ = env_logger::try_init(); | ||
| let mut server = mockito::Server::new(); | ||
| let mut server = mockito::Server::new_async().await; | ||
| let _mock = server |
There was a problem hiding this comment.
Don't we need to change from create() to create_async() in this file?
let _mock = server
.mock("GET", "/hello")
.match_header("Authorization", "Bearer test-token")
.with_status(204)
.create(); // shouldn't it be `.create_async().await;` ?There was a problem hiding this comment.
Looking at the crate documentation, there doesn't seem to be much difference between the two, i.e. both will work here, but using .create_async we don't block and await, which would maybe have performance implications when registering many mocks, but in our use case there's no difference as long as so the difference is if we started any other tasks before, calling .create also works.create() would block the only worker thread and block these tasks as well, but only for a short time that it takes to register a mock, so there's not much difference in our case.
Nevermind, didn't read the whole thing, it seems blocking the runtime for any amount of time is a hard error somehow.
From mockito's docs:
Mockito comes with both a sync and an async interface.
In order to write async tests, you’ll need to use the _async methods:
- Server::new_async
- Server::new_with_opts_async
- Mock::create_async
- Mock::assert_async
- Mock::matched_async
- Mock::remove_async
…otherwise your tests will not compile and you’ll see the following error:
Cannot block the current thread from within a runtime.
This happens because a function attempted to block the current thread while the thread is being used to drive asynchronous tasks.
In this case you're right, we need to use .create_async() everywhere
| assert_eq!( | ||
| error.map(|s| s.replace(char::is_numeric, "")), // ignore the task id | ||
| Some("task panicked".to_string()) | ||
| Some("task panicked with message \"Oh dear\"".to_string()) |
There was a problem hiding this comment.
typo: there's two spaces in task panicked
There was a problem hiding this comment.
The whole commit has been dropped
aa17bd1 to
4a069ce
Compare
| let mut server = mockito::Server::new_async().await; | ||
| let _mock = server | ||
| .mock("GET", "/not-a-known-url") | ||
| .with_status(404) |
There was a problem hiding this comment.
You missed to apply create_async to the line below. (line 899)
There was a problem hiding this comment.
:-) I will fixed that.
That said this demonstrates that @Bravo555 comment is correct. The tests are working as expected with create() as well ass with create_async().
Codecov ReportAttention: Patch coverage is Additional details and impacted files📢 Thoughts on this report? Let us know! |
With `mockito 1.5`, one has to use the `new_async` and `create_async` methods. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
2dbf2b1 to
a76659c
Compare
Proposed changes
Types of changes
Paste Link to the issue
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments