Bug 33584 - Fix failures for Database/Commenter.t on MySQL 8
Summary: Fix failures for Database/Commenter.t on MySQL 8
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 32334
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-21 15:28 UTC by Marcel de Rooy
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 33584: Fix failures for Commenter.t on MySQL8 (2.91 KB, patch)
2023-04-21 17:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info (1.37 KB, patch)
2023-04-24 12:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33584: Fix failures for Commenter.t on MySQL8 (2.88 KB, patch)
2023-04-24 14:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info (1.33 KB, patch)
2023-04-24 14:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33584: Fix failures for Commenter.t on MySQL8 (2.93 KB, patch)
2023-04-24 14:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info (1.39 KB, patch)
2023-04-24 14:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33612: Fix cat-search.inc and remove home-search.inc (6.16 KB, patch)
2023-04-25 21:37 UTC, Lucas Gass
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-04-21 15:28:37 UTC
# Subtest: ->new, dry_run
    1..6
    ok 1 - No dbh
    ok 2 - dbh no string
    ok 3 - Schema deleted
    not ok 4 - No comment for toc_request
    not ok 5 - Numbered comment for toc_request
    not ok 6 - Reset for one-columned article_requests
not ok 1 - ->new, dry_run

22:24:01 koha_1       |     #   Failed test 'No comment for toc_request'
22:24:01 koha_1       |     #   at t/db_dependent/Koha/Database/Commenter.t line 34.
22:24:01 koha_1       |     #          got: '0'
22:24:01 koha_1       |     #     expected: '1'
22:24:01 koha_1       | Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
=>         [sub renumber] next if $params->{table} && $col->{table_name} ne $params->{table};

Looks like we are reading info from another database or so, here:

SELECT table_name, column_name, column_comment FROM information_schema.columns
WHERE table_schema=? AND table_name=?
ORDER BY table_name, column_name|;

Or perhaps no permissions ?
Comment 1 Marcel de Rooy 2023-04-21 15:29:34 UTC
Failures second subtest should be related to problem in first:

# Subtest: ->clear, ->reset, ->renumber
    1..6
    not ok 1 - Found no comment for sometext
    not ok 2 - Found no comment for anotherint
    ok 3 - Found comment for sometext
    ok 4 - Found comment for anotherint
    not ok 5 - Found Column_1
    not ok 6 - Found Column_5
not ok 2 - ->clear, ->reset, ->renumber
Comment 2 Marcel de Rooy 2023-04-21 17:15:34 UTC
Created attachment 150068 [details] [review]
Bug 33584: Fix failures for Commenter.t on MySQL8

The column names from information_schema are uppercase now in MySQL8.
In the arrayref with sliced hashes, we expected lowercase.

See e.g. https://stackoverflow.com/questions/54538448/what-are-the-changes-in-mysql-8-result-rowset-case

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t
Do it again if possible with a MySQL 8 db server.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2023-04-21 17:16:12 UTC
Tomas: Could you please try to run the test with Mysql 8 ? Or just push and see :)
Comment 4 Marcel de Rooy 2023-04-21 17:17:10 UTC
(In reply to Marcel de Rooy from comment #0)
> Looks like we are reading info from another database or so, here:
> 
> SELECT table_name, column_name, column_comment FROM
> information_schema.columns
> WHERE table_schema=? AND table_name=?
> ORDER BY table_name, column_name|;
> 
> Or perhaps no permissions ?

It is always simpler: I got uppercase in my hash keys instead of lower case.
Comment 5 David Nind 2023-04-24 10:34:14 UTC
I tested using KTD (changed database in .env file from DB_IMAGE=mariadb:10.5 to DB_IMAGE=mysql:8).

I also tested using the default database (mariadb:10.5) - the tests pass before and after the patch is applied.

Before patch applied (MySQL 8)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Before the patch was applied, I got these results:

prove t/db_dependent/Koha/Database/Commenter.t
t/db_dependent/Koha/Database/Commenter.t .. 
    #   Failed test 'No comment for toc_request'
    #   at t/db_dependent/Koha/Database/Commenter.t line 34.
    #          got: '0'
    #     expected: '1'
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.

    #   Failed test 'Numbered comment for toc_request'
    #   at t/db_dependent/Koha/Database/Commenter.t line 39.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Reset for one-columned article_requests'
    #   at t/db_dependent/Koha/Database/Commenter.t line 49.
    #                   'ALTER TABLE koha_kohadev.article_requests MODIFY COLUMN `toc_request` tinyint NOT NULL DEFAULT 0 COMMENT 'borrower requested table of contents';'
    #     doesn't match '(?^um:ALTER.*toc_request.*DEFAULT 0;(.*\n)+ALTER.*toc_request.*COMMENT.*$)'
    # Looks like you failed 3 tests of 6.
t/db_dependent/Koha/Database/Commenter.t .. 1/2 
#   Failed test '->new, dry_run'
#   at t/db_dependent/Koha/Database/Commenter.t line 52.

    #   Failed test 'Found no comment for sometext'
    #   at t/db_dependent/Koha/Database/Commenter.t line 61.
    #          got: 'some nice quote's'
    #     expected: ''

    #   Failed test 'Found no comment for anotherint'
    #   at t/db_dependent/Koha/Database/Commenter.t line 62.
    #          got: 'my int'
    #     expected: ''
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.
Use of uninitialized value in string ne at /kohadevbox/koha/Koha/Database/Commenter.pm line 147.

    #   Failed test 'Found Column_1'
    #   at t/db_dependent/Koha/Database/Commenter.t line 73.
    #          got: 'my int'
    #     expected: 'Column_1'

    #   Failed test 'Found Column_5'
    #   at t/db_dependent/Koha/Database/Commenter.t line 74.
    #          got: ''
    #     expected: 'Column_5'
    # Looks like you failed 4 tests of 6.

#   Failed test '->clear, ->reset, ->renumber'
#   at t/db_dependent/Koha/Database/Commenter.t line 78.
# Looks like you failed 2 tests of 2.
t/db_dependent/Koha/Database/Commenter.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Database/Commenter.t (Wstat: 512 Tests: 2 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 2
Files=1, Tests=2,  1 wallclock secs ( 0.01 usr  0.00 sys +  0.50 cusr  0.03 csys =  0.54 CPU)
Result: FAIL

After the patch was applied (MySQL 8)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After the patch was applied, I got these results:

prove t/db_dependent/Koha/Database/Commenter.t
t/db_dependent/Koha/Database/Commenter.t .. 1/2 Failure for database_commenter_1:timestamp2 at /kohadevbox/koha/Koha/Database/Commenter.pm line 266.

    #   Failed test 'Found Column_5'
    #   at t/db_dependent/Koha/Database/Commenter.t line 80.
    #          got: ''
    #     expected: 'Column_5'
    # Looks like you failed 1 test of 6.

#   Failed test '->clear, ->reset, ->renumber'
#   at t/db_dependent/Koha/Database/Commenter.t line 84.
# Looks like you failed 1 test of 2.
t/db_dependent/Koha/Database/Commenter.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/2 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Database/Commenter.t (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=2,  1 wallclock secs ( 0.02 usr  0.00 sys +  0.50 cusr  0.06 csys =  0.58 CPU)
Result: FAIL
Comment 6 Marcel de Rooy 2023-04-24 12:36:35 UTC
Created attachment 150154 [details] [review]
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info

MySQL 8 returns this additional information for a timestamp such
as the timestamp2 in the Commenter test.
Which results in the following alter:
    ALTER TABLE koha_myclone.database_commenter_1 MODIFY COLUMN `timestamp2` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP DEFAULT_GENERATED COMMENT 'Column_5'
And that statement fails again on that keyword.
For now, it seems sufficient to ignore this keyword when building the alter statement.

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Did test it now under MySQL 8.
Comment 7 Marcel de Rooy 2023-04-24 12:40:21 UTC
(In reply to Marcel de Rooy from comment #3)
> Tomas: Could you please try to run the test with Mysql 8 ? Or just push and
> see :)

Tested it now with a Mysql 8 container myself :)
Comment 8 Tomás Cohen Arazi 2023-04-24 14:44:06 UTC
I tested with:

$ DB_IMAGE=mysql:8 ktd up -d
$ ktd --shell
$ prove t/db_dependent/Koha/Database/Commenter.t

and 

$ ktd down
$ DB_IMAGE=mariadb:10.5 ktd up -d
$ ktd --shell
$ prove t/db_dependent/Koha/Database/Commenter.t

In both cases it run fine.
Comment 9 Tomás Cohen Arazi 2023-04-24 14:44:41 UTC
Created attachment 150159 [details] [review]
Bug 33584: Fix failures for Commenter.t on MySQL8

The column names from information_schema are uppercase now in MySQL8.
In the arrayref with sliced hashes, we expected lowercase.

See e.g. https://stackoverflow.com/questions/54538448/what-are-the-changes-in-mysql-8-result-rowset-case

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t
Do it again if possible with a MySQL 8 db server.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Tomás Cohen Arazi 2023-04-24 14:44:44 UTC
Created attachment 150160 [details] [review]
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info

MySQL 8 returns this additional information for a timestamp such
as the timestamp2 in the Commenter test.
Which results in the following alter:
    ALTER TABLE koha_myclone.database_commenter_1 MODIFY COLUMN `timestamp2` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP DEFAULT_GENERATED COMMENT 'Column_5'
And that statement fails again on that keyword.
For now, it seems sufficient to ignore this keyword when building the alter statement.

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Did test it now under MySQL 8.
Comment 11 Tomás Cohen Arazi 2023-04-24 14:46:36 UTC
Created attachment 150161 [details] [review]
Bug 33584: Fix failures for Commenter.t on MySQL8

The column names from information_schema are uppercase now in MySQL8.
In the arrayref with sliced hashes, we expected lowercase.

See e.g. https://stackoverflow.com/questions/54538448/what-are-the-changes-in-mysql-8-result-rowset-case

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t
Do it again if possible with a MySQL 8 db server.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2023-04-24 14:46:39 UTC
Created attachment 150162 [details] [review]
Bug 33584: (follow-up) Remove DEFAULT_GENERATED from column info

MySQL 8 returns this additional information for a timestamp such
as the timestamp2 in the Commenter test.
Which results in the following alter:
    ALTER TABLE koha_myclone.database_commenter_1 MODIFY COLUMN `timestamp2` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP DEFAULT_GENERATED COMMENT 'Column_5'
And that statement fails again on that keyword.
For now, it seems sufficient to ignore this keyword when building the alter statement.

Test plan:
Run t/db_dependent/Koha/Database/Commenter.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Did test it now under MySQL 8.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2023-04-24 15:34:52 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Lucas Gass 2023-04-25 21:37:46 UTC Comment hidden (obsolete)
Comment 15 David Nind 2023-04-26 00:13:32 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize 2023-05-11 10:55:27 UTC
Bugfix to enhancement not in 22.11.x. Marking as resolved.