Bug 31473 - Test about bad OpacHiddenItems conf fragile
Summary: Test about bad OpacHiddenItems conf fragile
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords: rel_21_05_candidate, rel_21_11_candidate
Depends on: 28125
Blocks:
  Show dependency treegraph
 
Reported: 2022-08-26 19:39 UTC by Tomás Cohen Arazi
Modified: 2022-10-23 09:59 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.06,21.11.12,21.05.19


Attachments
Bug 31473: Fix fragile test about bad OpacHiddenItems conf (1.75 KB, patch)
2022-08-26 19:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-08-26 19:39:16 UTC
It tests for data truncation (when a two-letter string is passed for a tinyint(1) field) but there's been some behavior change in the way expressions are processed that makes the test fail:

#   Failed test at t/db_dependent/Koha/Items.t line 1754.
#          got: 'Truncated incorrect DECIMAL value: 'AB''
#     expected: 'Truncated incorrect DOUBLE value: 'AB''

The fact the expression was tested as a DECIMAL or a DOUBLE first is not really relevant here.
Comment 1 Tomás Cohen Arazi 2022-08-26 19:53:51 UTC
Created attachment 139849 [details] [review]
Bug 31473: Fix fragile test about bad OpacHiddenItems conf

There's been a behavior change in recent MariaDB that made some cases
deal with truncated data in DOUBLE context instead of DECIMAL.

Probably this:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-27380

This made the affected test fail consistently on those versions.

This trick for checking OpacHiddenItems is correct (as introduced on bug
28125 doesn't really make use of the warning, just prints it in
about.pl) so one option was to just check for a warning.

I decided to keep the test, but add the optional DECIMAL|DOUBLE check on
the qr. This way other eventual changes will make it fail and serve as a
warning in case something more relevant changes.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Items.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2022-08-26 19:56:42 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 3 Victor Grousset/tuxayo 2022-08-29 21:08:55 UTC
Thanks a lot :D

Backport needed down to 21.05.x
Comment 4 Victor Grousset/tuxayo 2022-09-08 03:33:25 UTC
Disregarding the backport cascade workflow. Not a problem with this patch. Now test can pass again!

Backported: Pushed to 21.05.x branch for 21.05.19
Comment 5 Lucas Gass 2022-10-04 16:30:35 UTC
Backported to 22.05.x for 22.05.06
Comment 6 Victor Grousset/tuxayo 2022-10-17 00:28:13 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 7 Arthur Suzuki 2022-10-22 22:10:44 UTC
Thanks!

Pushed to 21.11 for 21.11.12