-
Notifications
You must be signed in to change notification settings - Fork 102
Bump meilisearch to v0.25.0 - New task API #225
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,9 +24,9 @@ jobs: | |||||
| - name: Build | ||||||
| run: cargo build --verbose | ||||||
| - name: Meilisearch (latest version) setup with Docker | ||||||
| run: docker run -d -p 7700:7700 getmeili/meilisearch:latest ./meilisearch --no-analytics=true --master-key=masterKey | ||||||
| run: docker run -d -p 7700:7700 getmeili/meilisearch:latest ./meilisearch --no-analytics --master-key=masterKey | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not yet, will be for v0.26.0, not v0.25.0 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure that I understand that, the previous version of meilisearch works with this change, and the newer version (v0.26) will only work with this change. This patch makes the command line compatible with the future version of Meilisearch and doesn't break the v0.25 one. Can't we accept this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum from what I see it has been merged some times ago already; meilisearch/meilisearch#1984 |
||||||
| - name: Run tests | ||||||
| run: cargo test --verbose -- --test-threads=1 | ||||||
| run: cargo test --verbose | ||||||
irevoire marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| linter: | ||||||
| name: clippy-check | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| [package] | ||
| name = "meilisearch-test-macro" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
|
|
||
| [lib] | ||
| proc-macro = true | ||
|
|
||
| # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html | ||
|
|
||
| [dependencies] | ||
| proc-macro2 = "1.0.0" | ||
| quote = "1.0.0" | ||
| syn = { version = "1.0.0", features = ["clone-impls", "full", "parsing", "printing", "proc-macro"], default-features = false } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # Meilisearch test macro | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ ❤️ ❤️ |
||
|
|
||
| This crate defines the `meilisearch_test` macro. | ||
|
|
||
| Since the code is a little bit harsh to read, here is a complete explanation of how to use it. | ||
| The macro aims to ease the writing of tests by: | ||
| 1. Reducing the amount of code you need to write and maintain for each test. | ||
| 2. Ensuring All your indexes as a unique name so they can all run in parallel. | ||
| 3. Ensuring you never forget to delete your index if you need one. | ||
|
|
||
|
|
||
| Before explaining its usage, we're going to see a simple test *before* this macro: | ||
| ```rust | ||
| #[async_test] | ||
| async fn test_get_all_tasks() -> Result<(), Error> { | ||
| let client = Client::new("http://localhost:7700", "masterKey"); | ||
|
|
||
| let index = client | ||
| .create_index("test_get_all_tasks", None) | ||
| .await? | ||
| .wait_for_completion(&client, None, None) | ||
| .await? | ||
| .try_make_index(&client) | ||
| .unwrap(); | ||
|
|
||
| let tasks = index.get_all_tasks().await?; | ||
| // The only task is the creation of the index | ||
| assert_eq!(status.len(), 1); | ||
|
|
||
| index.delete() | ||
| .await? | ||
| .wait_for_completion(&client, None, None) | ||
| .await?; | ||
| Ok(()) | ||
| } | ||
| ``` | ||
|
|
||
| I have multiple problems with this test: | ||
| - `let client = Client::new("http://localhost:7700", "masterKey");`: This line is always the same in every test. | ||
| And if you make a typo on the http addr or the master key, you'll have an error. | ||
| - `let index = client.create_index("test_get_all_tasks", None)...`: Each test needs to have an unique name. | ||
| This means we currently need to write the name of the test everywhere; it's not practical. | ||
| - There are 11 lines dedicated to the creation and deletion of the index; this is once again something that'll never change | ||
| whatever the test is. But, if you ever forget to delete the index at the end, you'll get in some trouble to re-run | ||
| the tests. | ||
|
|
||
| ------- | ||
|
|
||
| With this macro, all these problems are solved. See a rewrite of this test: | ||
| ```rust | ||
| #[meilisearch_test] | ||
| async fn test_get_all_tasks(index: Index, client: Client) -> Result<(), Error> { | ||
| let tasks = index.get_all_tasks().await?; | ||
| // The only task is the creation of the index | ||
| assert_eq!(status.len(), 1); | ||
| } | ||
| ``` | ||
|
|
||
| So now you're probably seeing what happened. By using an index and a client in the parameter of | ||
| the test, the macro automatically did the same thing we've seen before. | ||
| There are a few rules, though: | ||
| 1. The macro only handles three types of arguments: | ||
| - `String`: It returns the name of the test. | ||
| - `Client`: It creates a client like that: `Client::new("http://localhost:7700", "masterKey")`. | ||
| - `Index`: It creates and deletes an index, as we've seen before. | ||
| 2. You only get what you asked for. That means if you don't ask for an index, no index will be created in meilisearch. | ||
| So, if you are testing the creation of indexes, you can ask for a `Client` and a `String` and then create it yourself. | ||
| The index won't be present in meilisearch. | ||
| 3. You can put your parameters in the order you want it won't change anything. | ||
| 4. Everything you use **must** be in scope directly. If you're using an `Index`, you must write `Index` in the parameters, | ||
| not `meilisearch_rust::Index` or `crate::Index`. | ||
| 5. And I think that's all, use and abuse it 🎉 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| #![warn(clippy::pedantic)] | ||
| #![recursion_limit = "4096"] | ||
|
|
||
| extern crate proc_macro; | ||
|
|
||
| use proc_macro::TokenStream; | ||
| use proc_macro2::Span; | ||
| use quote::quote; | ||
| use syn::*; | ||
|
|
||
| #[proc_macro_attribute] | ||
| pub fn meilisearch_test(params: TokenStream, input: TokenStream) -> TokenStream { | ||
| assert!( | ||
| params.is_empty(), | ||
| "the #[async_test] attribute currently does not take parameters" | ||
| ); | ||
|
|
||
| let mut inner = parse_macro_input!(input as Item); | ||
| let mut outer = inner.clone(); | ||
| if let (&mut Item::Fn(ref mut inner_fn), &mut Item::Fn(ref mut outer_fn)) = | ||
| (&mut inner, &mut outer) | ||
| { | ||
| inner_fn.sig.ident = Ident::new( | ||
| &("_inner_meilisearch_test_macro_".to_string() + &inner_fn.sig.ident.to_string()), | ||
| Span::call_site(), | ||
| ); | ||
| let inner_ident = &inner_fn.sig.ident; | ||
| inner_fn.vis = Visibility::Inherited; | ||
| inner_fn.attrs.clear(); | ||
| assert!( | ||
| outer_fn.sig.asyncness.take().is_some(), | ||
| "#[meilisearch_test] can only be applied to async functions" | ||
| ); | ||
|
|
||
| #[derive(Debug, PartialEq, Eq)] | ||
| enum Param { | ||
| Client, | ||
| Index, | ||
| String, | ||
| } | ||
|
|
||
| let mut params = Vec::new(); | ||
|
|
||
| let parameters = &inner_fn.sig.inputs; | ||
| for param in parameters.iter() { | ||
| match param { | ||
| FnArg::Typed(PatType { ty, .. }) => match &**ty { | ||
| Type::Path(TypePath { path: Path { segments, .. }, .. } ) if segments.last().unwrap().ident.to_string() == "String" => { | ||
| params.push(Param::String); | ||
| } | ||
| Type::Path(TypePath { path: Path { segments, .. }, .. } ) if segments.last().unwrap().ident.to_string() == "Index" => { | ||
| params.push(Param::Index); | ||
| } | ||
| Type::Path(TypePath { path: Path { segments, .. }, .. } ) if segments.last().unwrap().ident.to_string() == "Client" => { | ||
| params.push(Param::Client); | ||
| } | ||
| // TODO: throw this error while pointing to the specific token | ||
| ty => panic!( | ||
| "#[meilisearch_test] can only receive Client, Index or String as parameters but received {:?}", ty | ||
| ), | ||
| }, | ||
| // TODO: throw this error while pointing to the specific token | ||
| // Used `self` as a parameter | ||
| _ => panic!( | ||
| "#[meilisearch_test] can only receive Client, Index or String as parameters" | ||
| ), | ||
| } | ||
| } | ||
|
|
||
| // if a `Client` or an `Index` was asked for the test we must create a meilisearch `Client`. | ||
| let use_client = params | ||
| .iter() | ||
| .any(|param| matches!(param, Param::Client | Param::Index)); | ||
| // if a `String` or an `Index` was asked then we need to extract the name of the test function. | ||
| let use_name = params | ||
| .iter() | ||
| .any(|param| matches!(param, Param::String | Param::Index)); | ||
| let use_index = params.contains(&Param::Index); | ||
|
|
||
| // Now we are going to build the body of the outer function | ||
| let mut outer_block: Vec<Stmt> = Vec::new(); | ||
|
|
||
| // First we need to check if a client will be used and create it if it’s the case | ||
| if use_client { | ||
| outer_block.push(parse_quote!( | ||
| let client = Client::new("http://localhost:7700", "masterKey"); | ||
| )); | ||
| } | ||
|
|
||
| // Now we do the same for the index name | ||
| if use_name { | ||
| let name = &outer_fn.sig.ident; | ||
| outer_block.push(parse_quote!( | ||
| let name = stringify!(#name).to_string(); | ||
| )); | ||
| } | ||
|
|
||
| // And finally if an index was asked we create it and wait until meilisearch confirm its creation. | ||
| // We’ll need to delete it later. | ||
| if use_index { | ||
| outer_block.push(parse_quote!( | ||
| let index = client | ||
| .create_index(&name, None) | ||
| .await | ||
| .unwrap() | ||
| .wait_for_completion(&client, None, None) | ||
| .await | ||
| .unwrap() | ||
| .try_make_index(&client) | ||
| .unwrap(); | ||
| )); | ||
| } | ||
|
|
||
| // Create a list of params separated by comma with the name we defined previously. | ||
| let params: Vec<Expr> = params | ||
| .into_iter() | ||
| .map(|param| match param { | ||
| Param::Client => parse_quote!(client), | ||
| Param::Index => parse_quote!(index), | ||
| Param::String => parse_quote!(name), | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Now we can call the user code with our parameters :tada: | ||
| outer_block.push(parse_quote!( | ||
| let result = #inner_ident(#(#params.clone()),*).await; | ||
| )); | ||
|
|
||
| // And right before the end, if an index was created we need to delete it. | ||
| if use_index { | ||
| outer_block.push(parse_quote!( | ||
| index | ||
| .delete() | ||
| .await | ||
| .unwrap() | ||
| .wait_for_completion(&client, None, None) | ||
| .await | ||
| .unwrap(); | ||
| )); | ||
| } | ||
|
|
||
| // Finally, for the great finish we just return the result the user gave us. | ||
| outer_block.push(parse_quote!(return result;)); | ||
|
|
||
| outer_fn.sig.inputs.clear(); | ||
| outer_fn.sig.asyncness = inner_fn.sig.asyncness.clone(); | ||
| outer_fn | ||
| .attrs | ||
| .push(parse_quote!(#[futures_await_test::async_test])); | ||
| outer_fn.block.stmts = outer_block; | ||
| } else { | ||
| panic!("#[meilisearch_test] can only be applied to async functions") | ||
| } | ||
| quote!( | ||
| #inner | ||
| #outer | ||
| ) | ||
| .into() | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.