From f80ea24d2b581bd6c1a922992072f2f0873f8a77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9mentine=20Urquizar?= Date: Mon, 22 Mar 2021 19:17:18 +0100 Subject: [PATCH 1/3] Add tests on every platform and fix clippy errors --- .github/workflows/rust.yml | 46 ++++++++++++++++--- bors.toml | 7 ++- meilisearch-http/src/index/search.rs | 2 +- .../src/index_controller/index_actor/actor.rs | 2 +- .../update_actor/update_store.rs | 20 ++++---- meilisearch-http/src/routes/stats.rs | 3 +- meilisearch-http/tests/common/index.rs | 2 +- meilisearch-http/tests/common/server.rs | 2 +- meilisearch-http/tests/common/service.rs | 20 ++++---- meilisearch-http/tests/index/get_index.rs | 6 +-- meilisearch-http/tests/snapshot/mod.rs | 2 +- 11 files changed, 73 insertions(+), 39 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 230339c49..3ea29b253 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -13,12 +13,44 @@ env: jobs: tests: - - runs-on: ubuntu-latest - + name: Tests on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-18.04, macos-latest] steps: - uses: actions/checkout@v2 - - name: Build - run: cargo build --verbose - - name: Run tests - run: cargo test --verbose + - name: Run cargo test + uses: actions-rs/cargo@v1 + with: + command: test + args: --locked --release + + # We don't run test on Windows since we get the following error: There is not enough space on the disk. + build-on-windows: + name: Build on windows-latest + runs-on: windows-latest + steps: + - uses: actions/checkout@v2 + - name: Run cargo test + uses: actions-rs/cargo@v1 + with: + command: build + + clippy: + name: Run Clippy + runs-on: ubuntu-18.04 + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: clippy + - name: Run cargo clippy + uses: actions-rs/cargo@v1 + with: + command: clippy + args: --all-targets -- --deny warnings diff --git a/bors.toml b/bors.toml index dd19e1f5b..2e2da90ac 100644 --- a/bors.toml +++ b/bors.toml @@ -1,3 +1,8 @@ -status = ['tests'] +status = [ + 'Tests on ubuntu-18.04', + 'Tests on macos-latest', + 'Build on windows-latest', + 'Run Clippy' +] # 3 hours timeout timeout-sec = 10800 diff --git a/meilisearch-http/src/index/search.rs b/meilisearch-http/src/index/search.rs index 1cccfcd58..0ff6c1bc3 100644 --- a/meilisearch-http/src/index/search.rs +++ b/meilisearch-http/src/index/search.rs @@ -162,7 +162,7 @@ impl Index { ); for (_id, obkv) in self.documents(&rtxn, documents_ids)? { - let document = make_document(&all_attributes, &fields_ids_map, obkv.clone())?; + let document = make_document(&all_attributes, &fields_ids_map, obkv)?; let formatted = compute_formatted( &fields_ids_map, obkv, diff --git a/meilisearch-http/src/index_controller/index_actor/actor.rs b/meilisearch-http/src/index_controller/index_actor/actor.rs index c54b2edd2..9cca6557b 100644 --- a/meilisearch-http/src/index_controller/index_actor/actor.rs +++ b/meilisearch-http/src/index_controller/index_actor/actor.rs @@ -197,7 +197,7 @@ impl IndexActor { .map_err(|e| IndexError::Error(e.into())) } - *self.processing.write().await = Some(meta.index_uuid().clone()); + *self.processing.write().await = Some(*meta.index_uuid()); let result = get_result(self, meta, data).await; *self.processing.write().await = None; diff --git a/meilisearch-http/src/index_controller/update_actor/update_store.rs b/meilisearch-http/src/index_controller/update_actor/update_store.rs index 3d6c4e396..5cf00da34 100644 --- a/meilisearch-http/src/index_controller/update_actor/update_store.rs +++ b/meilisearch-http/src/index_controller/update_actor/update_store.rs @@ -13,16 +13,16 @@ use uuid::Uuid; use crate::helpers::EnvSizer; use crate::index_controller::updates::*; -type BEU64 = heed::zerocopy::U64; +type Beu64 = heed::zerocopy::U64; #[derive(Clone)] pub struct UpdateStore { pub env: Env, - pending_meta: Database, SerdeJson>>, - pending: Database, SerdeJson>, - processed_meta: Database, SerdeJson>>, - failed_meta: Database, SerdeJson>>, - aborted_meta: Database, SerdeJson>>, + pending_meta: Database, SerdeJson>>, + pending: Database, SerdeJson>, + processed_meta: Database, SerdeJson>>, + failed_meta: Database, SerdeJson>>, + aborted_meta: Database, SerdeJson>>, processing: Arc>>>, notification_sender: mpsc::Sender<()>, /// A lock on the update loop. This is meant to prevent a snapshot to occur while an update is @@ -176,7 +176,7 @@ where // asking for the id and registering it so other update registering // will be forced to wait for a new write txn. let update_id = self.new_update_id(&wtxn)?; - let update_key = BEU64::new(update_id); + let update_key = Beu64::new(update_id); let meta = Enqueued::new(meta, update_id, index_uuid); self.pending_meta.put(&mut wtxn, &update_key, &meta)?; @@ -295,7 +295,7 @@ where /// Returns the update associated meta or `None` if the update doesn't exist. pub fn meta(&self, update_id: u64) -> heed::Result>> { let rtxn = self.env.read_txn()?; - let key = BEU64::new(update_id); + let key = Beu64::new(update_id); if let Some(ref meta) = *self.processing.read() { if meta.id() == update_id { @@ -331,7 +331,7 @@ where #[allow(dead_code)] pub fn abort_update(&self, update_id: u64) -> heed::Result>> { let mut wtxn = self.env.write_txn()?; - let key = BEU64::new(update_id); + let key = Beu64::new(update_id); // We cannot abort an update that is currently being processed. if self.pending_meta.first(&wtxn)?.map(|(key, _)| key.get()) == Some(update_id) { @@ -369,7 +369,7 @@ where } for (id, aborted) in &aborted_updates { - let key = BEU64::new(*id); + let key = Beu64::new(*id); self.aborted_meta.put(&mut wtxn, &key, &aborted)?; self.pending_meta.delete(&mut wtxn, &key)?; self.pending.delete(&mut wtxn, &key)?; diff --git a/meilisearch-http/src/routes/stats.rs b/meilisearch-http/src/routes/stats.rs index 988e6cf40..226b62fcd 100644 --- a/meilisearch-http/src/routes/stats.rs +++ b/meilisearch-http/src/routes/stats.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::iter::FromIterator; use actix_web::get; use actix_web::web; @@ -33,7 +32,7 @@ impl From for IndexStatsResponse { Self { number_of_documents: stats.number_of_documents, is_indexing: stats.is_indexing, - fields_distribution: BTreeMap::from_iter(stats.fields_distribution.into_iter()), + fields_distribution: stats.fields_distribution.into_iter().collect(), } } } diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index aaf108cf6..86aa80c3d 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -47,7 +47,7 @@ impl Index<'_> { update_id as u64 } - pub async fn create<'a>(&'a self, primary_key: Option<&str>) -> (Value, StatusCode) { + pub async fn create(& self, primary_key: Option<&str>) -> (Value, StatusCode) { let body = json!({ "uid": self.uid, "primaryKey": primary_key, diff --git a/meilisearch-http/tests/common/server.rs b/meilisearch-http/tests/common/server.rs index e814ab3ed..100722ec4 100644 --- a/meilisearch-http/tests/common/server.rs +++ b/meilisearch-http/tests/common/server.rs @@ -44,7 +44,7 @@ impl Server { } /// Returns a view to an index. There is no guarantee that the index exists. - pub fn index<'a>(&'a self, uid: impl AsRef) -> Index<'a> { + pub fn index(& self, uid: impl AsRef) -> Index<'_> { Index { uid: encode(uid.as_ref()), service: &self.service, diff --git a/meilisearch-http/tests/common/service.rs b/meilisearch-http/tests/common/service.rs index b9bbffc05..08db5b9dc 100644 --- a/meilisearch-http/tests/common/service.rs +++ b/meilisearch-http/tests/common/service.rs @@ -8,13 +8,13 @@ pub struct Service(pub Data); impl Service { pub async fn post(&self, url: impl AsRef, body: Value) -> (Value, StatusCode) { - let mut app = test::init_service(create_app!(&self.0, true)).await; + let app = test::init_service(create_app!(&self.0, true)).await; let req = test::TestRequest::post() .uri(url.as_ref()) .set_json(&body) .to_request(); - let res = test::call_service(&mut app, req).await; + let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; @@ -28,14 +28,14 @@ impl Service { url: impl AsRef, body: impl AsRef, ) -> (Value, StatusCode) { - let mut app = test::init_service(create_app!(&self.0, true)).await; + let app = test::init_service(create_app!(&self.0, true)).await; let req = test::TestRequest::post() .uri(url.as_ref()) .set_payload(body.as_ref().to_string()) .insert_header(("content-type", "application/json")) .to_request(); - let res = test::call_service(&mut app, req).await; + let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; @@ -44,10 +44,10 @@ impl Service { } pub async fn get(&self, url: impl AsRef) -> (Value, StatusCode) { - let mut app = test::init_service(create_app!(&self.0, true)).await; + let app = test::init_service(create_app!(&self.0, true)).await; let req = test::TestRequest::get().uri(url.as_ref()).to_request(); - let res = test::call_service(&mut app, req).await; + let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; @@ -56,13 +56,13 @@ impl Service { } pub async fn put(&self, url: impl AsRef, body: Value) -> (Value, StatusCode) { - let mut app = test::init_service(create_app!(&self.0, true)).await; + let app = test::init_service(create_app!(&self.0, true)).await; let req = test::TestRequest::put() .uri(url.as_ref()) .set_json(&body) .to_request(); - let res = test::call_service(&mut app, req).await; + let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; @@ -71,10 +71,10 @@ impl Service { } pub async fn delete(&self, url: impl AsRef) -> (Value, StatusCode) { - let mut app = test::init_service(create_app!(&self.0, true)).await; + let app = test::init_service(create_app!(&self.0, true)).await; let req = test::TestRequest::delete().uri(url.as_ref()).to_request(); - let res = test::call_service(&mut app, req).await; + let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; diff --git a/meilisearch-http/tests/index/get_index.rs b/meilisearch-http/tests/index/get_index.rs index 8671d85cd..ce4d7b792 100644 --- a/meilisearch-http/tests/index/get_index.rs +++ b/meilisearch-http/tests/index/get_index.rs @@ -55,10 +55,8 @@ async fn list_multiple_indexes() { assert_eq!(arr.len(), 2); assert!(arr .iter() - .find(|entry| entry["uid"] == "test" && entry["primaryKey"] == Value::Null) - .is_some()); + .any(|entry| entry["uid"] == "test" && entry["primaryKey"] == Value::Null)); assert!(arr .iter() - .find(|entry| entry["uid"] == "test1" && entry["primaryKey"] == "key") - .is_some()); + .any(|entry| entry["uid"] == "test1" && entry["primaryKey"] == "key")); } diff --git a/meilisearch-http/tests/snapshot/mod.rs b/meilisearch-http/tests/snapshot/mod.rs index 36763a1ab..caed293e6 100644 --- a/meilisearch-http/tests/snapshot/mod.rs +++ b/meilisearch-http/tests/snapshot/mod.rs @@ -35,7 +35,7 @@ async fn perform_snapshot() { let snapshot_path = snapshot_dir .path() .to_owned() - .join(format!("db.snapshot")); + .join("db.snapshot".to_string()); let options = Opt { import_snapshot: Some(snapshot_path), From 1ba46f8f776409d550495c37591a9718a4404451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9mentine=20Urquizar?= Date: Tue, 27 Apr 2021 12:16:24 +0200 Subject: [PATCH 2/3] Disable clippy rule --- .../update_actor/update_store.rs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/meilisearch-http/src/index_controller/update_actor/update_store.rs b/meilisearch-http/src/index_controller/update_actor/update_store.rs index 5cf00da34..f1895829b 100644 --- a/meilisearch-http/src/index_controller/update_actor/update_store.rs +++ b/meilisearch-http/src/index_controller/update_actor/update_store.rs @@ -13,16 +13,17 @@ use uuid::Uuid; use crate::helpers::EnvSizer; use crate::index_controller::updates::*; -type Beu64 = heed::zerocopy::U64; +#[allow(clippy::upper_case_acronyms)] +type BEU64 = heed::zerocopy::U64; #[derive(Clone)] pub struct UpdateStore { pub env: Env, - pending_meta: Database, SerdeJson>>, - pending: Database, SerdeJson>, - processed_meta: Database, SerdeJson>>, - failed_meta: Database, SerdeJson>>, - aborted_meta: Database, SerdeJson>>, + pending_meta: Database, SerdeJson>>, + pending: Database, SerdeJson>, + processed_meta: Database, SerdeJson>>, + failed_meta: Database, SerdeJson>>, + aborted_meta: Database, SerdeJson>>, processing: Arc>>>, notification_sender: mpsc::Sender<()>, /// A lock on the update loop. This is meant to prevent a snapshot to occur while an update is @@ -176,7 +177,7 @@ where // asking for the id and registering it so other update registering // will be forced to wait for a new write txn. let update_id = self.new_update_id(&wtxn)?; - let update_key = Beu64::new(update_id); + let update_key = BEU64::new(update_id); let meta = Enqueued::new(meta, update_id, index_uuid); self.pending_meta.put(&mut wtxn, &update_key, &meta)?; @@ -295,7 +296,7 @@ where /// Returns the update associated meta or `None` if the update doesn't exist. pub fn meta(&self, update_id: u64) -> heed::Result>> { let rtxn = self.env.read_txn()?; - let key = Beu64::new(update_id); + let key = BEU64::new(update_id); if let Some(ref meta) = *self.processing.read() { if meta.id() == update_id { @@ -331,7 +332,7 @@ where #[allow(dead_code)] pub fn abort_update(&self, update_id: u64) -> heed::Result>> { let mut wtxn = self.env.write_txn()?; - let key = Beu64::new(update_id); + let key = BEU64::new(update_id); // We cannot abort an update that is currently being processed. if self.pending_meta.first(&wtxn)?.map(|(key, _)| key.get()) == Some(update_id) { @@ -369,7 +370,7 @@ where } for (id, aborted) in &aborted_updates { - let key = Beu64::new(*id); + let key = BEU64::new(*id); self.aborted_meta.put(&mut wtxn, &key, &aborted)?; self.pending_meta.delete(&mut wtxn, &key)?; self.pending.delete(&mut wtxn, &key)?; From 0c41adf868639e4970674b28e152381b81464178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9mentine=20Urquizar?= Date: Tue, 27 Apr 2021 12:36:36 +0200 Subject: [PATCH 3/3] Update CI --- .github/workflows/rust.yml | 18 ++++++++++++++---- bors.toml | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3ea29b253..bdbf50a95 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -21,6 +21,11 @@ jobs: os: [ubuntu-18.04, macos-latest] steps: - uses: actions/checkout@v2 + - name: Run cargo check without any default features + uses: actions-rs/cargo@v1 + with: + command: check + args: --no-default-features - name: Run cargo test uses: actions-rs/cargo@v1 with: @@ -28,15 +33,20 @@ jobs: args: --locked --release # We don't run test on Windows since we get the following error: There is not enough space on the disk. - build-on-windows: - name: Build on windows-latest + check-on-windows: + name: Cargo check on Windows runs-on: windows-latest steps: - uses: actions/checkout@v2 - - name: Run cargo test + - name: Run cargo check without any default features uses: actions-rs/cargo@v1 with: - command: build + command: check + args: --no-default-features + - name: Run cargo check with all default features + uses: actions-rs/cargo@v1 + with: + command: check clippy: name: Run Clippy diff --git a/bors.toml b/bors.toml index 2e2da90ac..45133f836 100644 --- a/bors.toml +++ b/bors.toml @@ -1,7 +1,7 @@ status = [ 'Tests on ubuntu-18.04', 'Tests on macos-latest', - 'Build on windows-latest', + 'Cargo check on Windows', 'Run Clippy' ] # 3 hours timeout