Bug 16868 - Silence error t/db_dependent/Linker_FirstMatch.t
Summary: Silence error t/db_dependent/Linker_FirstMatch.t
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Mark Tompsett
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 16155
Blocks:
  Show dependency treegraph
 
Reported: 2016-07-07 01:37 UTC by Mark Tompsett
Modified: 2017-12-07 22:16 UTC (History)
6 users (show)

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


Attachments
auth_header.sql (25.36 KB, text/plain)
2016-07-07 01:54 UTC, Mark Tompsett
Details
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (2.03 KB, patch)
2016-07-07 01:56 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (2.17 KB, patch)
2016-07-15 06:59 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16868: Jonathan Druart testing dbh code (1.02 KB, patch)
2016-07-16 14:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.44 KB, patch)
2016-07-16 15:00 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Jonathan Druart testing dbh code (1.04 KB, patch)
2016-07-16 15:03 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.44 KB, patch)
2016-07-16 15:04 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.82 KB, patch)
2016-07-16 15:28 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.66 KB, patch)
2016-07-21 00:25 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.74 KB, patch)
2016-07-26 16:24 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t (6.81 KB, patch)
2016-08-02 10:07 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2016-07-07 01:37:05 UTC
When the auth_header table has records which exclude 1xx and 2xx tags, the $bibfield doesn't match anything. This in turn sets it to undef, which triggers an error on the next line killing the test.

Solution: improve the SQL query so it will find the expected records, or not and properly skip tests.
Comment 1 Mark Tompsett 2016-07-07 01:54:47 UTC
Created attachment 53154 [details]
auth_header.sql

My auth_header which triggered this problem.
perhaps a:
use koha_library;
source auth_header.sql;
might be useful for testing. Don't push anywhere!
Comment 2 Mark Tompsett 2016-07-07 01:56:08 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2016-07-15 06:59:41 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-07-15 14:39:51 UTC
Instead of trying to fix specific case, I'd suggest to insert the data you want to make the tests pass:
- Start transaction
- Create the authority you want
- Retrieve the authority and use it for the tests.
Comment 5 Mark Tompsett 2016-07-15 16:42:00 UTC
(In reply to Jonathan Druart from comment #4)
> Instead of trying to fix specific case, I'd suggest to insert the data you
> want to make the tests pass:
> - Start transaction
> - Create the authority you want
> - Retrieve the authority and use it for the tests.

Okay, I'll refactor to use t::lib::Database (your code from bug 14334) and t::lib::TestBuilder.
Comment 6 Mark Tompsett 2016-07-16 14:59:45 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2016-07-16 15:00:30 UTC Comment hidden (obsolete)
Comment 8 Mark Tompsett 2016-07-16 15:03:58 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2016-07-16 15:04:04 UTC Comment hidden (obsolete)
Comment 10 Mark Tompsett 2016-07-16 15:08:26 UTC
Comment on attachment 53454 [details] [review]
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t

Review of attachment 53454 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/Linker_FirstMatch.t
@@ -36,3 @@
>      my $tag = $bibfield->tag();
> -    $tag =~ s/^./6/;
> -    $bibfield->update(tag => $tag);

For the record, this code never worked. It doesn't change the tag, it adds a subfield called tag.
Comment 11 Mark Tompsett 2016-07-16 15:28:37 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2016-07-20 23:24:32 UTC
Mark, could you please do something like this instead? (grabbed from some other test and adjusted to your use).

sub get_authtority_record {

    my $main_heading_field =
            ( C4::Context->preference('MARCFlavour') eq 'UNIMARC' )
                ? '200'
                : '100';

    my $auth = MARC::Record->new();
    $auth->append_fields(
        MARC::Field->new($main_heading_field, ' ', ' ',
                           a => 'Geisel, Theodor Seuss,',
                           d => '1904-1991' ));
    return $auth;
}

It seems to me that the interpolated XML stuff obfuscates the code, and does unneeded XML processing.

Also, the global transaction for tests was discouraged so no need to introduce Database.pm for this simple test as we just do the transaction as it is needed in the tests (think of subtests that do its own rollbacks, etc). So go for the usual:

use Koha::Database;
my $schema = Koha::Database->new->schema;

$schema->storage->txn_begin;

< tests here >

$schema->storage->txn_rollback;

BTW, it seems to me that this tests don't bother to launch zebra so they... don't... actually... test anything? Good examples on how to do this can be found (for C&P) on search_utf8.t <- This is not your fault and not really part of your bug report, but worth mentioning. The relevant test, is the last one, and it depends on zebra having the record indexed!! Notice C4::Headings::authorities() is calling _search() which actually triggers a Zebra search.
Comment 13 Mark Tompsett 2016-07-20 23:40:54 UTC
(In reply to Tomás Cohen Arazi from comment #12)
> Mark, could you please do something like ...
[SNIP]

Okay... I'll adjust based on your feedback.
Comment 14 Mark Tompsett 2016-07-21 00:25:41 UTC Comment hidden (obsolete)
Comment 15 Marc Véron 2016-07-26 16:24:55 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2016-08-02 10:07:24 UTC
Created attachment 53880 [details] [review]
Bug 16868: Silence error t/db_dependent/Linker_FirstMatch.t

When the auth_header table has records which exclude 1xx and
2xx tags, the $bibfield doesn't match anything. This in turn
sets it to undef, which triggers an error on the next line
killing the test.

This was completely refactored to provide the data necessary
for the tests to pass, and to be more comprehensive (both MARC
and UNIMARC are tested). The tests are then run.

C4::Headings::authorities is mocked, so that this test is not
dependent on a search engine.

TEST PLAN
---------
1) back up DB
2) DELETE FROM auth_header;
3) SOURCE auth_header.sql;
   -- the provided file
4) prove t/db_dependent/Linker_FirstMatch.t
   -- should barf before running all the tests
5) apply all patches
6) prove t/db_dependent/Linker_FirstMatch.t
   -- should work happy
7) run koha qa test tools
8) restore your backup

Followed test plan, behaves as expected.
Signed-off-by: Marc <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Brendan Gallagher 2016-08-04 20:04:51 UTC
Pushed to master.  Should be in the November 2016 release.  Thanks!
Comment 18 Frédéric Demians 2016-08-05 06:54:00 UTC
Pushed in 16.05. Will be in 16.05.03.