From 4c3f00a9eacd2b9b76de63ed3f23a1a8d31b5db9 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 7 Jul 2025 01:04:57 +0000 Subject: [PATCH] Bug 36947: (QA follow-up) Add skippable unit test for fi_FI locale This patch adds a skippable unit test for the fi_FI locale to prove that the patches work well for other locales that don't use the default Unicode collation. It will run on koha-testing-docker, but will be skipped on systems lacking it. This patch also fleshes out the existing unit tests a bit more to prove that the patches also improves the sorting of names with diacritics generally. That is, the default Unicode sort is better than the existing stringwise/bytewise sort even for English and French locales. Test plan: 0. prove -v t/Koha/SearchEngine/Elasticsearch/Search.t \ t/db_dependent/Search_FacetSorting.t --- t/Koha/SearchEngine/Elasticsearch/Search.t | 89 +++++++++++++++++----- t/db_dependent/Search_FacetSorting.t | 77 +++++++++++++++---- 2 files changed, 134 insertions(+), 32 deletions(-) diff --git a/t/Koha/SearchEngine/Elasticsearch/Search.t b/t/Koha/SearchEngine/Elasticsearch/Search.t index cab8c84401..06b1c157ed 100755 --- a/t/Koha/SearchEngine/Elasticsearch/Search.t +++ b/t/Koha/SearchEngine/Elasticsearch/Search.t @@ -18,7 +18,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 4; +use Test::More tests => 5; use Test::MockModule; use t::lib::Mocks; use Encode qw( encode ); @@ -31,18 +31,8 @@ use_ok('Koha::SearchEngine::Elasticsearch::Search'); subtest '_sort_facets' => sub { plan tests => 3; t::lib::Mocks::mock_preference( 'SearchEngine', 'Elasticsearch' ); - my $facets = [ - { facet_label_value => 'Mary' }, - { facet_label_value => 'Harry' }, - { facet_label_value => 'Fairy' }, - { facet_label_value => 'Ari' }, - { facet_label_value => 'mary' }, - { facet_label_value => 'harry' }, - { facet_label_value => 'fairy' }, - { facet_label_value => 'ari' }, - { facet_label_value => 'étienne' }, - { facet_label_value => 'Åuthor' }, - ]; + + my $facets = _get_facets(); my @normal_sort_facets = sort { $a->{facet_label_value} cmp $b->{facet_label_value} } @$facets; my @normal_expected_facets = ( @@ -50,12 +40,15 @@ subtest '_sort_facets' => sub { { facet_label_value => 'Fairy' }, { facet_label_value => 'Harry' }, { facet_label_value => 'Mary' }, + { facet_label_value => 'Zambidis' }, { facet_label_value => 'ari' }, { facet_label_value => 'fairy' }, { facet_label_value => 'harry' }, { facet_label_value => 'mary' }, + { facet_label_value => 'Åberg, Erik' }, { facet_label_value => 'Åuthor' }, { facet_label_value => 'étienne' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, ); #NOTE: stringwise/bytewise is not UTF-8 friendly @@ -68,6 +61,7 @@ subtest '_sort_facets' => sub { #is used for the locales of English (en) and French (fr). my $sorted_facets = $search->_sort_facets( { facets => $facets, locale => 'default' } ); my $expected = [ + { facet_label_value => 'Åberg, Erik' }, { facet_label_value => 'ari' }, { facet_label_value => 'Ari' }, { facet_label_value => 'Åuthor' }, @@ -78,6 +72,8 @@ subtest '_sort_facets' => sub { { facet_label_value => 'Harry' }, { facet_label_value => 'mary' }, { facet_label_value => 'Mary' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, ]; is_deeply( $sorted_facets, $expected, "Facets sorted correctly with default locale" ); @@ -91,13 +87,39 @@ subtest '_sort_facets' => sub { #NOTE: If "locale" is not provided to _sort_facets, it will look up the LC_COLLATE #for the local system. This is what allows this function to work well in production. #However, since LC_COLLATE could vary from system to system running these unit tests, - #we can't test arbitrary locales here. + #we can't test it reliably here. +}; - #TODO: It would be great to explicitly use a fi_FI locale for another test, but we cannot guarantee - #that every system has that locale. That said, perhaps we could do some conditional unit - #testing if certain locales are available when running tests. This could allow for the Koha community - #to thoroughly test this functionality, while letting anyone run the unit tests regardless of their - #installed locales. +subtest '_sort_facets_zebra with fi_FI locale' => sub { + plan tests => 1; + my $locale_map = _get_locale_map(); +SKIP: { + skip( "fi_FI.utf8 locale not available on this system", 1 ) unless $locale_map->{"fi_FI.utf8"}; + + my $facets = _get_facets(); + + my $search = Koha::SearchEngine::Elasticsearch::Search->new( + { index => $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX } ); + + # Test with explicit locale parameter + my $sorted_facets_explicit = $search->_sort_facets( { facets => $facets, locale => 'fi_FI' } ); + my $expected = [ + { facet_label_value => 'ari' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'Mary' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, + { facet_label_value => 'Åberg, Erik' }, + { facet_label_value => 'Åuthor' }, + ]; + is_deeply( $sorted_facets_explicit, $expected, "Zebra facets sorted correctly with explicit locale" ); + } }; subtest 'search_auth_compat' => sub { @@ -176,4 +198,33 @@ subtest 'search_auth_compat' => sub { is( @$results[0]->{series}, 1, 'Valid main heading with ShowHeadingUse' ); }; +sub _get_facets { + my $facets = [ + { facet_label_value => 'Mary' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'Åberg, Erik' }, + { facet_label_value => 'Åuthor' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'ari' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, + ]; + return $facets; +} + +sub _get_locale_map { + my $map = {}; + my @locales = `locale -a`; + foreach my $locale (@locales) { + chomp($locale); + $map->{$locale} = 1; + } + return $map; +} + 1; diff --git a/t/db_dependent/Search_FacetSorting.t b/t/db_dependent/Search_FacetSorting.t index 96383ad8a7..a65abc9171 100755 --- a/t/db_dependent/Search_FacetSorting.t +++ b/t/db_dependent/Search_FacetSorting.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 1; +use Test::NoWarnings; +use Test::More tests => 3; use Test::MockModule; use t::lib::Mocks; use C4::Search; @@ -27,22 +28,12 @@ use utf8; subtest '_sort_facets_zebra with system preference' => sub { plan tests => 3; - my $facets = [ - { facet_label_value => 'Mary' }, - { facet_label_value => 'Harry' }, - { facet_label_value => 'Fairy' }, - { facet_label_value => 'Ari' }, - { facet_label_value => 'mary' }, - { facet_label_value => 'harry' }, - { facet_label_value => 'fairy' }, - { facet_label_value => 'ari' }, - { facet_label_value => 'étienne' }, - { facet_label_value => 'Åuthor' }, - ]; + my $facets = _get_facets(); # Test with explicit locale parameter my $sorted_facets_explicit = C4::Search::_sort_facets_zebra( $facets, 'default' ); my $expected = [ + { facet_label_value => 'Åberg, Erik' }, { facet_label_value => 'ari' }, { facet_label_value => 'Ari' }, { facet_label_value => 'Åuthor' }, @@ -53,6 +44,8 @@ subtest '_sort_facets_zebra with system preference' => sub { { facet_label_value => 'Harry' }, { facet_label_value => 'mary' }, { facet_label_value => 'Mary' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, ]; is_deeply( $sorted_facets_explicit, $expected, "Zebra facets sorted correctly with explicit locale" ); @@ -66,3 +59,61 @@ subtest '_sort_facets_zebra with system preference' => sub { my $sorted_facets_fallback = C4::Search::_sort_facets_zebra($facets); is( ref($sorted_facets_fallback), 'ARRAY', "Zebra facets sorting fallback works" ); }; + +subtest '_sort_facets_zebra with fi_FI locale' => sub { + plan tests => 1; + my $locale_map = _get_locale_map(); +SKIP: { + skip( "fi_FI.utf8 locale not available on this system", 1 ) unless $locale_map->{"fi_FI.utf8"}; + + my $facets = _get_facets(); + + # Test with explicit locale parameter + my $sorted_facets_explicit = C4::Search::_sort_facets_zebra( $facets, 'fi_FI' ); + my $expected = [ + { facet_label_value => 'ari' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'Mary' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, + { facet_label_value => 'Åberg, Erik' }, + { facet_label_value => 'Åuthor' }, + ]; + is_deeply( $sorted_facets_explicit, $expected, "Zebra facets sorted correctly with explicit locale" ); + } +}; + +sub _get_facets { + my $facets = [ + { facet_label_value => 'Mary' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'Åberg, Erik' }, + { facet_label_value => 'Åuthor' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'ari' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'Šostakovitš, Dmitri' }, + { facet_label_value => 'Zambidis' }, + ]; + return $facets; +} + +sub _get_locale_map { + my $map = {}; + my @locales = `locale -a`; + foreach my $locale (@locales) { + chomp($locale); + $map->{$locale} = 1; + } + return $map; +} -- 2.39.5