Bugzilla – Attachment 183818 Details for
Bug 36947
Sort Elasticsearch/Zebra facets according to configurable locale instead of using Perl's stringwise/bytewise sort
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36947: (QA follow-up) Add skippable unit test for fi_FI locale
Bug-36947-QA-follow-up-Add-skippable-unit-test-for.patch (text/plain), 10.91 KB, created by
David Cook
on 2025-07-07 01:10:44 UTC
(
hide
)
Description:
Bug 36947: (QA follow-up) Add skippable unit test for fi_FI locale
Filename:
MIME Type:
Creator:
David Cook
Created:
2025-07-07 01:10:44 UTC
Size:
10.91 KB
patch
obsolete
>From 4c3f00a9eacd2b9b76de63ed3f23a1a8d31b5db9 Mon Sep 17 00:00:00 2001 >From: David Cook <dcook@prosentient.com.au> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 36947
:
167110
|
167125
|
167141
|
167142
|
167143
|
167145
|
167146
|
167188
|
167198
|
167880
|
168488
|
170489
|
183551
|
183552
|
183553
|
183563
|
183564
|
183565
|
183566
|
183567
|
183568
|
183726
|
183727
|
183817
|
183818
|
183881
|
183882
|
183883
|
183884
|
183885
|
183886
|
183887