From b64cd2a3e361d43c67d612d045b22b6feeba56b5 Mon Sep 17 00:00:00 2001 From: many Date: Tue, 8 Jun 2021 14:11:00 +0200 Subject: [PATCH] Resolve PR comments --- milli/tests/search/mod.rs | 13 ++----- milli/tests/search/query_criteria.rs | 53 ++++++++++++++-------------- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/milli/tests/search/mod.rs b/milli/tests/search/mod.rs index 60ad3d45f..98102f9e9 100644 --- a/milli/tests/search/mod.rs +++ b/milli/tests/search/mod.rs @@ -52,13 +52,6 @@ pub fn setup_search_index_with_criteria(criteria: &[Criterion]) -> Index { index } -#[allow(dead_code)] -pub fn external_to_internal_ids(index: &Index, external_ids: &[&str]) -> Vec { - let mut rtxn = index.read_txn().unwrap(); - let docid_map = index.external_documents_ids(&mut rtxn).unwrap(); - external_ids.iter().map(|id| docid_map.get(id).unwrap()).collect() -} - pub fn internal_to_external_ids(index: &Index, internal_ids: &[DocumentId]) -> Vec { let mut rtxn = index.read_txn().unwrap(); let docid_map = index.external_documents_ids(&mut rtxn).unwrap(); @@ -70,7 +63,7 @@ fn fetch_dataset() -> Vec { serde_json::Deserializer::from_str(CONTENT).into_iter().map(|r| r.unwrap()).collect() } -pub fn expected_order(criteria: &[Criterion], autorize_typo: bool, optional_words: bool) -> Vec { +pub fn expected_order(criteria: &[Criterion], authorize_typo: bool, optional_words: bool) -> Vec { let dataset = fetch_dataset(); let mut groups: Vec> = vec![dataset]; @@ -112,11 +105,11 @@ pub fn expected_order(criteria: &[Criterion], autorize_typo: bool, optional_word groups = std::mem::take(&mut new_groups); } - if autorize_typo && optional_words { + if authorize_typo && optional_words { groups.into_iter().flatten().collect() } else if optional_words { groups.into_iter().flatten().filter(|d| d.typo_rank == 0).collect() - } else if autorize_typo { + } else if authorize_typo { groups.into_iter().flatten().filter(|d| d.word_rank == 0).collect() } else { groups.into_iter().flatten().filter(|d| d.word_rank == 0 && d.typo_rank == 0).collect() diff --git a/milli/tests/search/query_criteria.rs b/milli/tests/search/query_criteria.rs index 7e398fceb..160b77c5b 100644 --- a/milli/tests/search/query_criteria.rs +++ b/milli/tests/search/query_criteria.rs @@ -23,36 +23,34 @@ macro_rules! test_criterion { search.authorize_typos($authorize_typos); search.optional_words($optional_word); - let SearchResult { matching_words: _matching_words, candidates: _candidates, documents_ids } = search.execute().unwrap(); + let SearchResult { documents_ids, .. } = search.execute().unwrap(); - let expected_external_ids: Vec<_> = search::expected_order(&criteria, $authorize_typos, $optional_word).into_iter().map(|d| d.id).collect(); + let expected_external_ids: Vec<_> = search::expected_order(&criteria, $authorize_typos, $optional_word) + .into_iter() + .map(|d| d.id).collect(); let documents_ids = search::internal_to_external_ids(&index, &documents_ids); - assert_eq!(documents_ids, expected_external_ids); - } } } - - -test_criterion!(none_allow_typo, ALLOW_OPTIONAL_WORDS, ALLOW_TYPOS); -test_criterion!(none_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS); -test_criterion!(words_allow_typo, ALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Words); -test_criterion!(attribute_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Attribute); -test_criterion!(attribute_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Attribute); -test_criterion!(exactness_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Exactness); -test_criterion!(exactness_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Exactness); -test_criterion!(proximity_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Proximity); -test_criterion!(proximity_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Proximity); -test_criterion!(asc_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Asc(S("asc_desc_rank"))); -test_criterion!(asc_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Asc(S("asc_desc_rank"))); -test_criterion!(desc_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Desc(S("asc_desc_rank"))); -test_criterion!(desc_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Desc(S("asc_desc_rank"))); -test_criterion!(asc_unexisting_field_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Asc(S("unexisting_field"))); -test_criterion!(asc_unexisting_field_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Asc(S("unexisting_field"))); -test_criterion!(desc_unexisting_field_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Desc(S("unexisting_field"))); -test_criterion!(desc_unexisting_field_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Desc(S("unexisting_field"))); +test_criterion!(none_allow_typo, ALLOW_OPTIONAL_WORDS, ALLOW_TYPOS); +test_criterion!(none_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS); +test_criterion!(words_allow_typo, ALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Words); +test_criterion!(attribute_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Attribute); +test_criterion!(attribute_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Attribute); +test_criterion!(exactness_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Exactness); +test_criterion!(exactness_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Exactness); +test_criterion!(proximity_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Proximity); +test_criterion!(proximity_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Proximity); +test_criterion!(asc_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Asc(S("asc_desc_rank"))); +test_criterion!(asc_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Asc(S("asc_desc_rank"))); +test_criterion!(desc_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Desc(S("asc_desc_rank"))); +test_criterion!(desc_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Desc(S("asc_desc_rank"))); +test_criterion!(asc_unexisting_field_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Asc(S("unexisting_field"))); +test_criterion!(asc_unexisting_field_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Asc(S("unexisting_field"))); +test_criterion!(desc_unexisting_field_allow_typo, DISALLOW_OPTIONAL_WORDS, ALLOW_TYPOS, Desc(S("unexisting_field"))); +test_criterion!(desc_unexisting_field_disallow_typo, DISALLOW_OPTIONAL_WORDS, DISALLOW_TYPOS, Desc(S("unexisting_field"))); #[test] fn criteria_mixup() { @@ -61,6 +59,7 @@ fn criteria_mixup() { let criteria_mix = { + // Criterion doesn't implement Copy, we create a new Criterion using a closure let desc = || Desc(S("asc_desc_rank")); // all possible criteria order vec![ @@ -199,12 +198,12 @@ fn criteria_mixup() { let mut search = Search::new(&mut rtxn, &index); search.query(search::TEST_QUERY); search.limit(EXTERNAL_DOCUMENTS_IDS.len()); - search.optional_words(true); - search.authorize_typos(true); + search.optional_words(ALLOW_OPTIONAL_WORDS); + search.authorize_typos(ALLOW_TYPOS); - let SearchResult { matching_words: _matching_words, candidates: _candidates, documents_ids } = search.execute().unwrap(); + let SearchResult { documents_ids, .. } = search.execute().unwrap(); - let expected_external_ids: Vec<_> = search::expected_order(&criteria, true, true).into_iter().map(|d| d.id).collect(); + let expected_external_ids: Vec<_> = search::expected_order(&criteria, ALLOW_OPTIONAL_WORDS, ALLOW_TYPOS).into_iter().map(|d| d.id).collect(); let documents_ids = search::internal_to_external_ids(&index, &documents_ids); assert_eq!(documents_ids, expected_external_ids);