From 2495058a6ec889a0e160f8ef69496590893431b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Wed, 15 Nov 2023 10:21:30 +0100 Subject: [PATCH] Fix the tests --- milli/src/boost.rs | 108 +------------------------------------- milli/tests/search/mod.rs | 9 ++++ 2 files changed, 10 insertions(+), 107 deletions(-) diff --git a/milli/src/boost.rs b/milli/src/boost.rs index 5571722bf..f90a68ee2 100644 --- a/milli/src/boost.rs +++ b/milli/src/boost.rs @@ -11,7 +11,7 @@ use crate::RankingRuleError; /// You must always cast it to a sort error or a criterion error. #[derive(Error, Debug)] pub enum BoostError { - #[error("Invalid syntax for the boost parameter: expected expression ending by `boost:`, found `{name}`.")] + #[error("Invalid syntax for the boost parameter: expected expression starting with `boost:`, found `{name}`.")] InvalidSyntax { name: String }, } @@ -26,12 +26,6 @@ impl From for RankingRuleError { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct Boost(pub String); -impl Boost { - pub fn filter(&self) -> &str { - &self.0 - } -} - impl FromStr for Boost { type Err = BoostError; @@ -42,103 +36,3 @@ impl FromStr for Boost { } } } - -#[cfg(test)] -mod tests { - use big_s::S; - use BoostError::*; - - use super::*; - - #[test] - fn parse_asc_desc() { - let valid_req = [ - ("truc:asc", Asc(Field(S("truc")))), - ("bidule:desc", Desc(Field(S("bidule")))), - ("a-b:desc", Desc(Field(S("a-b")))), - ("a:b:desc", Desc(Field(S("a:b")))), - ("a12:asc", Asc(Field(S("a12")))), - ("42:asc", Asc(Field(S("42")))), - ("_geoPoint(42, 59):asc", Asc(Geo([42., 59.]))), - ("_geoPoint(42.459, 59):desc", Desc(Geo([42.459, 59.]))), - ("_geoPoint(42, 59.895):desc", Desc(Geo([42., 59.895]))), - ("_geoPoint(42, 59.895):desc", Desc(Geo([42., 59.895]))), - ("_geoPoint(90.000000000, 180):desc", Desc(Geo([90., 180.]))), - ("_geoPoint(-90, -180.0000000000):asc", Asc(Geo([-90., -180.]))), - ("_geoPoint(42.0002, 59.895):desc", Desc(Geo([42.0002, 59.895]))), - ("_geoPoint(42., 59.):desc", Desc(Geo([42., 59.]))), - ("truc(12, 13):desc", Desc(Field(S("truc(12, 13)")))), - ]; - - for (req, expected) in valid_req { - let res = req.parse::(); - assert!( - res.is_ok(), - "Failed to parse `{}`, was expecting `{:?}` but instead got `{:?}`", - req, - expected, - res - ); - assert_eq!(res.unwrap(), expected); - } - - let invalid_req = [ - ("truc:machin", InvalidSyntax { name: S("truc:machin") }), - ("truc:deesc", InvalidSyntax { name: S("truc:deesc") }), - ("truc:asc:deesc", InvalidSyntax { name: S("truc:asc:deesc") }), - ("42desc", InvalidSyntax { name: S("42desc") }), - ("_geoPoint:asc", ReservedKeyword { name: S("_geoPoint") }), - ("_geoDistance:asc", ReservedKeyword { name: S("_geoDistance") }), - ("_geoPoint(42.12 , 59.598)", InvalidSyntax { name: S("_geoPoint(42.12 , 59.598)") }), - ( - "_geoPoint(42.12 , 59.598):deesc", - InvalidSyntax { name: S("_geoPoint(42.12 , 59.598):deesc") }, - ), - ( - "_geoPoint(42.12 , 59.598):machin", - InvalidSyntax { name: S("_geoPoint(42.12 , 59.598):machin") }, - ), - ( - "_geoPoint(42.12 , 59.598):asc:aasc", - InvalidSyntax { name: S("_geoPoint(42.12 , 59.598):asc:aasc") }, - ), - ( - "_geoPoint(42,12 , 59,598):desc", - ReservedKeyword { name: S("_geoPoint(42,12 , 59,598)") }, - ), - ("_geoPoint(35, 85, 75):asc", ReservedKeyword { name: S("_geoPoint(35, 85, 75)") }), - ("_geoPoint(18):asc", ReservedKeyword { name: S("_geoPoint(18)") }), - ("_geoPoint(200, 200):asc", GeoError(BadGeoError::Lat(200.))), - ("_geoPoint(90.000001, 0):asc", GeoError(BadGeoError::Lat(90.000001))), - ("_geoPoint(0, -180.000001):desc", GeoError(BadGeoError::Lng(-180.000001))), - ("_geoPoint(159.256, 130):asc", GeoError(BadGeoError::Lat(159.256))), - ("_geoPoint(12, -2021):desc", GeoError(BadGeoError::Lng(-2021.))), - ("_geo(12, -2021):asc", ReservedKeyword { name: S("_geo(12, -2021)") }), - ("_geo(12, -2021):desc", ReservedKeyword { name: S("_geo(12, -2021)") }), - ("_geoDistance(12, -2021):asc", ReservedKeyword { name: S("_geoDistance(12, -2021)") }), - ( - "_geoDistance(12, -2021):desc", - ReservedKeyword { name: S("_geoDistance(12, -2021)") }, - ), - ]; - - for (req, expected_error) in invalid_req { - let res = req.parse::(); - assert!( - res.is_err(), - "Should no be able to parse `{}`, was expecting an error but instead got: `{:?}`", - req, - res, - ); - let res = res.unwrap_err(); - assert_eq!( - res.to_string(), - expected_error.to_string(), - "Bad error for input {}: got `{:?}` instead of `{:?}`", - req, - res, - expected_error - ); - } - } -} diff --git a/milli/tests/search/mod.rs b/milli/tests/search/mod.rs index 0ac26004c..2dd413ad7 100644 --- a/milli/tests/search/mod.rs +++ b/milli/tests/search/mod.rs @@ -147,6 +147,15 @@ pub fn expected_order( new_groups .extend(group.linear_group_by_key(|d| d.asc_desc_rank).map(Vec::from)); } + RankingRule::Boost(filter) => { + // move the matching documents first, then the ones that don't match + group.sort_by_key(|d| if execute_filter(filter, d).is_some() { 0 } else { 1 }); + new_groups.extend( + group + .linear_group_by_key(|d| execute_filter(filter, d).is_some()) + .map(Vec::from), + ); + } RankingRule::Asc(_) | RankingRule::Desc(_) | RankingRule::Sort => { new_groups.push(group.clone()) }