ronantakizawa/codereview-bench huggingface dataset:
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/search/mod.rs (Rust)

Code before:
                  },
                  {
                    "prout": "truc 123980612321",
                    "machin": "lol 12345645333447879"
                  }
                ]
              }
            ]
            "###);
        },
    )
    .await;

    // Test typo search
    test_settings_documents_indexing_swapping_and_search(
        &documents,
        &json!({
            "searchableAttributes": ["title", "nested.object", "nested.machin"],
            "typoTolerance": {
              "enabled": true,
              "disableOnNumbers": true
            }
        }),
        &json!({"q": "123457"}),
        |response, code| {
            assert_eq!(code, 200, "{}", response);
            snapshot!(json_string!(response["hits"]), @r###"[]"###);
        },
    )
    .await;
}


Review comment (suggestion):
```suggestion
            assert_eq!(code, 200, "{response}");
```

Code after:
        &documents,
        &json!({
            "searchableAttributes": ["title", "nested.object", "nested.machin"],
            "typoTolerance": {
              "enabled": true,
              "disableOnNumbers": true
            }
        }),
        &json!({"q": "12345"}),
        |response, code| {
            assert_eq!(code, 200, "{response}");
            snapshot!(json_string!(response["hits"]), @r###"
            [
              {
                "id": 2,
                "title": "The second document 3398499",
                "nested": [
                  "array",
                  {
                    "object": "field 23245121,23223"
                  },
                  {
                    "prout": "truc 123980612321",
                    "machin": "lol 12345645333447879"
                  }
                ]
              }
            ]
            "###);
        },
    )
    .await;

    // Test typo search
    test_settings_documents_indexing_swapping_and_search(
        &documents,
        &json!({
            "searchableAttributes": ["title", "nested.object", "nested.machin"],
            "typoTolerance": {
              "enabled": true,
              "disableOnNumbers": true
            }
        }),
        &json!({"q": "123457"}),
        |response, code| {
            assert_eq!(code, 200, "{response}");
            snapshot!(json_string!(response["hits"]), @r###"[]"###);
        },
    )
    .await;
}
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/index/update_index.rs (Rust)

Code before:
use time::format_description::well_known::Rfc3339;
use time::OffsetDateTime;

use crate::common::encoder::Encoder;
use crate::common::Server;
use crate::json;

#[actix_rt::test]
async fn update_primary_key() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, code) = index.create(None).await;

    assert_eq!(code, 202);
    index.wait_task(task.uid()).await.succeeded();

    let (task, _status_code) = index.update(Some("primary")).await;

    let response = index.wait_task(task.uid()).await.succeeded();

    assert_eq!(response["status"], "succeeded");

    let (response, code) = index.get().await;

    assert_eq!(code, 200);

    assert_eq!(response["uid"], index.uid);
    assert!(response.get("createdAt").is_some());
    assert!(response.get("updatedAt").is_some());

    let created_at =
        OffsetDateTime::parse(response["createdAt"].as_str().unwrap(), &Rfc3339).unwrap();
    let updated_at =
        OffsetDateTime::parse(response["updatedAt"].as_str().unwrap(), &Rfc3339).unwrap();
    assert!(created_at < updated_at);

    assert_eq!(response["primaryKey"], "primary");
    assert_eq!(response.as_object().unwrap().len(), 4);
}

#[actix_rt::test]
async fn create_and_update_with_different_encoding() {
    let server = Server::new_shared();
    let index = server.unique_index_with_encoder(Encoder::Gzip);

Review comment (suggestion):
```suggestion
    let response = index.wait_task(task.uid()).await.succeeded();
```

Nice we don't even need this check now 😁 

Code after:
use time::format_description::well_known::Rfc3339;
use time::OffsetDateTime;

use crate::common::encoder::Encoder;
use crate::common::{shared_does_not_exists_index, shared_index_with_documents, Server};
use crate::json;

#[actix_rt::test]
async fn update_primary_key() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, code) = index.create(None).await;

    assert_eq!(code, 202);
    index.wait_task(task.uid()).await.succeeded();

    let (task, _status_code) = index.update(Some("primary")).await;
    index.wait_task(task.uid()).await.succeeded();

    let (response, code) = index.get().await;

    assert_eq!(code, 200);

    assert_eq!(response["uid"], index.uid);
    assert!(response.get("createdAt").is_some());
    assert!(response.get("updatedAt").is_some());

    let created_at =
        OffsetDateTime::parse(response["createdAt"].as_str().unwrap(), &Rfc3339).unwrap();
    let updated_at =
        OffsetDateTime::parse(response["updatedAt"].as_str().unwrap(), &Rfc3339).unwrap();
    assert!(created_at < updated_at);

    assert_eq!(response["primaryKey"], "primary");
    assert_eq!(response.as_object().unwrap().len(), 4);
}

#[actix_rt::test]
async fn create_and_update_with_different_encoding() {
    let server = Server::new_shared();
    let index = server.unique_index_with_encoder(Encoder::Gzip);
    let (create_task, code) = index.create(None).await;

    assert_eq!(code, 202);
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/index/update_index.rs (Rust)

Code before:

    let index = index.with_encoder(Encoder::Brotli);
    let (task, _status_code) = index.update(Some("primary")).await;

    let response = index.wait_task(task.uid()).await.succeeded();

    assert_eq!(response["status"], "succeeded");
}

#[actix_rt::test]
async fn update_nothing() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task1, code) = index.create(None).await;

    assert_eq!(code, 202);

    index.wait_task(task1.uid()).await.succeeded();

    let (task2, code) = index.update(None).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task2.uid()).await.succeeded();

    assert_eq!(response["status"], "succeeded");
}

#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (create_task, code) = index.create(Some("id")).await;

    assert_eq!(code, 202);
    index.wait_task(create_task.uid()).await.succeeded();

    let documents = json!([
        {
            "id": "11",
            "content": "foobar"
        }
    ]);
    let (add_docs_task, add_docs_status_code) = index.add_documents(documents, None).await;
    assert_eq!(add_docs_status_code, 202);
    index.wait_task(add_docs_task.uid()).await.succeeded();

    let (update_task, code) = index.update(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

Review comment (suggestion):
```suggestion
    index.wait_task(task2.uid()).await.succeeded();
```

Code after:

    index.wait_task(task.uid()).await.succeeded();
}

#[actix_rt::test]
async fn update_nothing() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task1, code) = index.create(None).await;

    assert_eq!(code, 202);

    index.wait_task(task1.uid()).await.succeeded();

    let (task2, code) = index.update(None).await;

    assert_eq!(code, 202);

    index.wait_task(task2.uid()).await.succeeded();
}

#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let index = shared_index_with_documents().await;

    let (update_task, code) = index.update_index_fail(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}`: Index already has a primary key: `id`.", index.uid),
        "code": "index_primary_key_already_exists",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists"
    });

    assert_eq!(response["error"], expected_response);
}

#[actix_rt::test]
async fn error_update_unexisting_index() {
    let index = shared_does_not_exists_index().await;
    let (task, code) = index.update_index_fail(Some("my-primary-key")).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}` not found.", index.uid),
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/index/update_index.rs (Rust)

Code before:

    assert_eq!(code, 202);

    let response = index.wait_task(task2.uid()).await.succeeded();

    assert_eq!(response["status"], "succeeded");
}

#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (create_task, code) = index.create(Some("id")).await;

    assert_eq!(code, 202);
    index.wait_task(create_task.uid()).await.succeeded();

    let documents = json!([
        {
            "id": "11",
            "content": "foobar"
        }
    ]);
    let (add_docs_task, add_docs_status_code) = index.add_documents(documents, None).await;
    assert_eq!(add_docs_status_code, 202);
    index.wait_task(add_docs_task.uid()).await.succeeded();

    let (update_task, code) = index.update(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}`: Index already has a primary key: `id`.", index.uid),
        "code": "index_primary_key_already_exists",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists"
    });

    assert_eq!(response["error"], expected_response);
}

#[actix_rt::test]
async fn error_update_unexisting_index() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, code) = index.update(None).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task.uid()).await.failed();

Review comment (suggestion):
In this case instead of creating a now index and pushing documents I think we can use one of the already existing indexes like the one with movies (SHARED_DOCUMENTS) crates/meilisearch/tests/common/mod.rs:228

Code after:

#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let index = shared_index_with_documents().await;

    let (update_task, code) = index.update_index_fail(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}`: Index already has a primary key: `id`.", index.uid),
        "code": "index_primary_key_already_exists",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists"
    });

    assert_eq!(response["error"], expected_response);
}

#[actix_rt::test]
async fn error_update_unexisting_index() {
    let index = shared_does_not_exists_index().await;
    let (task, code) = index.update_index_fail(Some("my-primary-key")).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}` not found.", index.uid),
        "code": "index_not_found",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_not_found"
    });

    assert_eq!(response["error"], expected_response);
}
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/index/update_index.rs (Rust)

Code before:
    assert_eq!(code, 202);

    let response = index.wait_task(task2.uid()).await.succeeded();

    assert_eq!(response["status"], "succeeded");
}

#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (create_task, code) = index.create(Some("id")).await;

    assert_eq!(code, 202);
    index.wait_task(create_task.uid()).await.succeeded();

    let documents = json!([
        {
            "id": "11",
            "content": "foobar"
        }
    ]);
    let (add_docs_task, add_docs_status_code) = index.add_documents(documents, None).await;
    assert_eq!(add_docs_status_code, 202);
    index.wait_task(add_docs_task.uid()).await.succeeded();

    let (update_task, code) = index.update(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}`: Index already has a primary key: `id`.", index.uid),
        "code": "index_primary_key_already_exists",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists"
    });

    assert_eq!(response["error"], expected_response);
}

#[actix_rt::test]
async fn error_update_unexisting_index() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, code) = index.update(None).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task.uid()).await.failed();


Review comment (suggestion):
Same here, we could use the special nonexistent index that already exists instead of making a unique index

Code after:
#[actix_rt::test]
async fn error_update_existing_primary_key() {
    let index = shared_index_with_documents().await;

    let (update_task, code) = index.update_index_fail(Some("primary")).await;

    assert_eq!(code, 202);
    let response = index.wait_task(update_task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}`: Index already has a primary key: `id`.", index.uid),
        "code": "index_primary_key_already_exists",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists"
    });

    assert_eq!(response["error"], expected_response);
}

#[actix_rt::test]
async fn error_update_unexisting_index() {
    let index = shared_does_not_exists_index().await;
    let (task, code) = index.update_index_fail(Some("my-primary-key")).await;

    assert_eq!(code, 202);

    let response = index.wait_task(task.uid()).await.failed();

    let expected_response = json!({
        "message": format!("Index `{}` not found.", index.uid),
        "code": "index_not_found",
        "type": "invalid_request",
        "link": "https://docs.meilisearch.com/errors#index_not_found"
    });

    assert_eq!(response["error"], expected_response);
}
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/tasks/mod.rs (Rust)

Code before:
      "batchUid": "[batch_uid]",
      "indexUid": null,
      "status": "succeeded",
      "type": "taskDeletion",
      "canceledBy": null,
      "details": {
        "matchedTasks": 1,
        "deletedTasks": 1,
        "originalFilter": "?uids=0"
      },
      "error": null,
      "duration": "[duration]",
      "enqueuedAt": "[date]",
      "startedAt": "[date]",
      "finishedAt": "[date]"
    }
    "###);
}

#[actix_web::test]
async fn test_summarized_dump_creation() {
    let server = Server::new_shared();
    let (task, _status_code) = server.create_dump().await;
    server.wait_task(task.uid()).await;
    let (task, _) = server.get_task(task.uid()).await;
    snapshot!(task,
        @r###"
    {
      "uid": "[uid]",
      "batchUid": "[batch_uid]",
      "indexUid": null,
      "status": "succeeded",
      "type": "dumpCreation",
      "canceledBy": null,
      "details": {
        "dumpUid": "[dumpUid]"
      },
      "error": null,
      "duration": "[duration]",
      "enqueuedAt": "[date]",
      "startedAt": "[date]",
      "finishedAt": "[date]"
    }
    "###);
}


Review comment (suggestion):
My bad, we cannot do it here for the dump 🙈 

Or you could update the Display implementation of our custom Value type to ignore the field `. details.dumpUid`

Code after:
      "batchUid": "[batch_uid]",
      "indexUid": null,
      "status": "succeeded",
      "type": "taskDeletion",
      "canceledBy": null,
      "details": {
        "matchedTasks": 1,
        "deletedTasks": 1,
        "originalFilter": "?uids=0"
      },
      "error": null,
      "duration": "[duration]",
      "enqueuedAt": "[date]",
      "startedAt": "[date]",
      "finishedAt": "[date]"
    }
    "###);
}

#[actix_web::test]
async fn test_summarized_dump_creation() {
    let server = Server::new_shared();
    let (task, _status_code) = server.create_dump().await;
    server.wait_task(task.uid()).await;
    let (task, _) = server.get_task(task.uid()).await;
    snapshot!(task,
        @r###"
    {
      "uid": "[uid]",
      "batchUid": "[batch_uid]",
      "indexUid": null,
      "status": "succeeded",
      "type": "dumpCreation",
      "canceledBy": null,
      "details": {
        "dumpUid": "[dump_uid]"
      },
      "error": null,
      "duration": "[duration]",
      "enqueuedAt": "[date]",
      "startedAt": "[date]",
      "finishedAt": "[date]"
    }
    "###);
}
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/documents/delete_documents.rs (Rust)

Code before:
use meili_snap::{json_string, snapshot};

use crate::common::{GetAllDocumentsOptions, Server};
use crate::json;

#[actix_rt::test]
async fn delete_one_document_unexisting_index() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, code) = index.delete_document(0).await;
    assert_eq!(code, 202);

    index.wait_task(task.uid()).await.failed();
}

#[actix_rt::test]
async fn delete_one_unexisting_document() {
    let server = Server::new_shared();
    let index = server.unique_index();
    index.create(None).await;
    let (response, code) = index.delete_document(0).await;
    assert_eq!(code, 202, "{response}");
    index.wait_task(response.uid()).await.succeeded();
}

#[actix_rt::test]
async fn delete_one_document() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, _status_code) =
        index.add_documents(json!([{ "id": 0, "content": "foobar" }]), None).await;
    index.wait_task(task.uid()).await.succeeded();
    let (task, status_code) = index.delete_document(0).await;
    assert_eq!(status_code, 202);

Review comment (suggestion):
Here we can use the missing index directly instead of getting a unique index and not initializing it

Code after:
use meili_snap::{json_string, snapshot};

use crate::common::{shared_does_not_exists_index, GetAllDocumentsOptions, Server};
use crate::json;

#[actix_rt::test]
async fn delete_one_document_unexisting_index() {
    let index = shared_does_not_exists_index().await;
    let (task, code) = index.delete_document_by_filter_fail(json!({"filter": "a = b"})).await;
    assert_eq!(code, 202);

    index.wait_task(task.uid()).await.failed();
}

#[actix_rt::test]
async fn delete_one_unexisting_document() {
    let server = Server::new_shared();
    let index = server.unique_index();
    index.create(None).await;
    let (response, code) = index.delete_document(0).await;
    assert_eq!(code, 202, "{response}");
    index.wait_task(response.uid()).await.succeeded();
}

#[actix_rt::test]
async fn delete_one_document() {
    let server = Server::new_shared();
    let index = server.unique_index();
    let (task, _status_code) =
        index.add_documents(json!([{ "id": 0, "content": "foobar" }]), None).await;
    index.wait_task(task.uid()).await.succeeded();
    let (task, status_code) = index.delete_document(0).await;
    assert_eq!(status_code, 202);
    index.wait_task(task.uid()).await.succeeded();
Repository: meilisearch/meilisearch
File: crates/meilisearch/tests/snapshot/mod.rs (Rust)

Code before:
    let (task, code) = server.create_snapshot().await;
    snapshot!(code, @"202 Accepted");
    snapshot!(json_string!(task, { ".enqueuedAt" => "[date]" }), @r###"
    {
      "taskUid": 0,
      "indexUid": null,
      "status": "enqueued",
      "type": "snapshotCreation",
      "enqueuedAt": "[date]"
    }
    "###);
    server.wait_task(task.uid()).await.succeeded();

    let temp = tempfile::tempdir().unwrap();
    let snapshot_path = snapshot_dir.path().to_owned().join("db.snapshot");

    let options = Opt { import_snapshot: Some(snapshot_path), ..default_settings(temp.path()) };
    let snapshot_server = Server::new_with_options(options).await.unwrap();

    // The snapshot creation task should NOT be spawned again => task is succeeded
    let (task, code) = snapshot_server.get_task(task.uid()).await;
    snapshot!(code, @"200 OK");
    snapshot!(json_string!(task, { ".enqueuedAt" => "[date]" }), @r#"
    {
      "uid": 0,
      "batchUid": 0,
      "indexUid": null,
      "status": "succeeded",
      "type": "snapshotCreation",
      "canceledBy": null,
      "error": null,
      "duration": null,
      "enqueuedAt": "[date]",
      "startedAt": null,
      "finishedAt": null
    }
    "#);
}


Review comment (performance):
I don't understand where this batch_id comes from.

To be sure we did everything properly, could you also snapshot the associated batch please?

Code after:
    let (task, code) = server.create_snapshot().await;
    snapshot!(code, @"202 Accepted");
    snapshot!(json_string!(task, { ".enqueuedAt" => "[date]" }), @r###"
    {
      "taskUid": 0,
      "indexUid": null,
      "status": "enqueued",
      "type": "snapshotCreation",
      "enqueuedAt": "[date]"
    }
    "###);
    server.wait_task(task.uid()).await.succeeded();

    let temp = tempfile::tempdir().unwrap();
    let snapshot_path = snapshot_dir.path().to_owned().join("db.snapshot");

    let options = Opt { import_snapshot: Some(snapshot_path), ..default_settings(temp.path()) };
    let snapshot_server = Server::new_with_options(options).await.unwrap();

    // The snapshot should have been taken without the snapshot creation task
    let (tasks, code) = snapshot_server.tasks().await;
    snapshot!(code, @"200 OK");
    snapshot!(tasks, @r#"
    {
      "results": [],
      "total": 0,
      "limit": 20,
      "from": null,
      "next": null
    }
    "#);

    // Ensure the task is not present in the snapshot
    let (task, code) = snapshot_server.get_task(0).await;
    snapshot!(code, @"404 Not Found");
    snapshot!(task, @r#"
    {
      "message": "Task `0` not found.",
      "code": "task_not_found",
      "type": "invalid_request",
      "link": "https://docs.meilisearch.com/errors#task_not_found"
    }
    "#);

    // Ensure the batch is also not present
    let (batch, code) = snapshot_server.get_batch(0).await;
    snapshot!(code, @"404 Not Found");
    snapshot!(batch, @r#"
    {
      "message": "Batch `0` not found.",
      "code": "batch_not_found",