This one does not work for me. I get the following output (with DEBUG on):

jcamins@kohadev:~/kohaclone$ perl t/db_dependent/Reserves.t 
1..4
ok 1 - use C4::Reserves;
# 
# Creating biblio instance for testing.
# Creating item instance for testing.
 INSERT INTO borrowers SET   borrowers.firstname = ? ,  borrowers.dateenrolled = ? ,  borrowers.userid = ? ,  borrowers.categorycode = ? ,  borrowers.surname = ? ,  borrowers.dateexpiry = ? ,  borrowers.password = ? ,  borrowers.cardnumber = ? ,  borrowers.branchcode = ? my firstname,2013-06-04,myfirstname.mysurname,S,my surname,2021-09-04,!,CARDNUMBER42,CPL at /home/jcamins/kohaclone/C4/SQLHelper.pm line 182.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 75 at /home/jcamins/kohaclone/C4/Members.pm line 556.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 75 at /home/jcamins/kohaclone/C4/Members.pm line 556.
not ok 2 - CheckReserves Test 1
#   Failed test 'CheckReserves Test 1'
#   at t/db_dependent/Reserves.t line 66.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 75 at /home/jcamins/kohaclone/C4/Members.pm line 556.
not ok 3 - CheckReserves Test 2
#   Failed test 'CheckReserves Test 2'
#   at t/db_dependent/Reserves.t line 69.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 75 at /home/jcamins/kohaclone/C4/Members.pm line 556.
not ok 4 - CheckReserves Test 3
#   Failed test 'CheckReserves Test 3'
#   at t/db_dependent/Reserves.t line 72.
# Deleting item testing instance.
# Deleting biblio testing instance.
# Deleting borrower.
# Looks like you failed 3 tests of 4.

Jared, thank you for testing.

I confirm the tests pass here :-/
Could you retry with the last patch please? It replaces ok with is. We should have a more readable failure.

(In reply to comment #4)
> Jared, thank you for testing.
> 
> I confirm the tests pass here :-/
> Could you retry with the last patch please? It replaces ok with is. We
> should have a more readable failure.

I'm still getting failures:
jcamins@kohadev:~/kohaclone$ prove t/db_dependent/Reserves.t 
t/db_dependent/Reserves.t .. 1/4 # 
# Creating biblio instance for testing.
# Creating item instance for testing.
 INSERT INTO borrowers SET   borrowers.firstname = ? ,  borrowers.dateenrolled = ? ,  borrowers.userid = ? ,  borrowers.categorycode = ? ,  borrowers.surname = ? ,  borrowers.dateexpiry = ? ,  borrowers.password = ? ,  borrowers.cardnumber = ? ,  borrowers.branchcode = ? my firstname,2013-06-07,myfirstname.mysurname,S,my surname,2021-09-07,!,CARDNUMBER42,CPL at /home/jcamins/kohaclone/C4/SQLHelper.pm line 182.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 78 at /home/jcamins/kohaclone/C4/Members.pm line 556.
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 78 at /home/jcamins/kohaclone/C4/Members.pm line 556.

#   Failed test 'CheckReserves Test 1'
#   at t/db_dependent/Reserves.t line 67.
#          got: ''
#     expected: 'Reserved'
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 78 at /home/jcamins/kohaclone/C4/Members.pm line 556.
t/db_dependent/Reserves.t .. 3/4 
#   Failed test 'CheckReserves Test 2'
#   at t/db_dependent/Reserves.t line 70.
#          got: ''
#     expected: 'Reserved'
SELECT borrowers.*, categories.category_type, categories.description
    FROM borrowers 
    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE borrowernumber = ? 78 at /home/jcamins/kohaclone/C4/Members.pm line 556.

#   Failed test 'CheckReserves Test 3'
#   at t/db_dependent/Reserves.t line 73.
#          got: ''
#     expected: 'Reserved'
# Deleting item testing instance.
# Deleting biblio testing instance.
# Deleting borrower.
# Looks like you failed 3 tests of 4.
t/db_dependent/Reserves.t .. Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/4 subtests 

Test Summary Report
-------------------
t/db_dependent/Reserves.t (Wstat: 768 Tests: 4 Failed: 3)
  Failed tests:  2-4
  Non-zero exit status: 3
Files=1, Tests=4,  1 wallclock secs ( 0.03 usr  0.01 sys +  0.60 cusr  0.06 csys =  0.70 CPU)
Result: FAIL

(In reply to comment #6)
> (In reply to comment #4)
> > Jared, thank you for testing.
> > 
> > I confirm the tests pass here :-/
> > Could you retry with the last patch please? It replaces ok with is. We
> > should have a more readable failure.
> 
> I'm still getting failures:

Yes, it is normal, I did not change the way to retrieve data.

I am really sorry but the output does not give information for debugging this.
I think it is caused by your data.
I use the database generated with the script provided on bug 10337 (depends on bug 10298). Like that the database just contains sample data.

(In reply to comment #6)
> I'm still getting failures:

Are these tests pass with the master's ut file?

(In reply to comment #8)
> (In reply to comment #6)
> > I'm still getting failures:
> 
> Are these tests pass with the master's ut file?

No. Isn't that the point of the patch, that the unit tests should pass even if there isn't any data for them?

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > I'm still getting failures:
> > 
> > Are these tests pass with the master's ut file?
> 
> No. Isn't that the point of the patch, that the unit tests should pass even
> if there isn't any data for them?

Yes of course, but in fact the only data I create is the borrower. So if the DB contains at least 1 borrower, this patch is useless.

Created attachment 19448 [details] [review]
Bug 10289: UT: Reserves.t needs to create its own data

Try before the patch:
prove t/db_dependent/Reserves.t

And after, it should produce:
  t/db_dependent/Reserves.t .. 1/4 #
  # Creating biblio instance for testing.
  # Creating item instance for testing.
  # Deleting item testing instance.
  # Deleting biblio testing instance.
  # Deleting borrower.
  t/db_dependent/Reserves.t .. ok
  All tests successful.
  Files=1, Tests=4,  1 wallclock secs ( 0.02 usr  0.01 sys +  0.39 cusr  0.02 csys =  0.44 CPU)
  Result: PASS

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

(In reply to Jonathan Druart from comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #6)
> > > > I'm still getting failures:
> > > 
> > > Are these tests pass with the master's ut file?
> > 
> > No. Isn't that the point of the patch, that the unit tests should pass even
> > if there isn't any data for them?
> 
> Yes of course, but in fact the only data I create is the borrower. So if the
> DB contains at least 1 borrower, this patch is useless.

That's a little too strong of a requirement, IMO, as I, for one, and I suspect most developers, would like to be able to run the DB-dependent tests in our normal testing databases.

I rather suspect what Jared is running into is that his test database already has a patron whose cardnumber is CARDNUMBER42.

I think that the requirement that tests should "create their own data" has a more general expression: "tests should make as few assumptions about the state of the database as possible".  What this means in this specific case is that the cardnumber for the patron created for this test should be set in such a way as to be guaranteed unique for that test run.

Pushed to master.  Thanks, Jonathan!

I've also pushed two follow-ups, one to wrap the tests in a transaction and another to remove the hard-coded patron cardnumber, which doesn't need to be set to any specific value for the tests to work.

(In reply to Galen Charlton from comment #13)
> > Yes of course, but in fact the only data I create is the borrower. So if the
> > DB contains at least 1 borrower, this patch is useless.
> That's a little too strong of a requirement, IMO, as I, for one, and I
> suspect most developers, would like to be able to run the DB-dependent tests
> in our normal testing databases.

Currently nobody is able to launch them, and the goal is to launch tests with sample data.
They don't insert patron, so I think we have to have some prerequisites.

> I rather suspect what Jared is running into is that his test database
> already has a patron whose cardnumber is CARDNUMBER42.

Yes, it was maybe to "generic" :)

(In reply to Galen Charlton from comment #14)
> Pushed to master.  Thanks, Jonathan!
> 
> I've also pushed two follow-ups, one to wrap the tests in a transaction and
> another to remove the hard-coded patron cardnumber, which doesn't need to be
> set to any specific value for the tests to work.

Thanks for the followups!
But now tests fail if a borrower exists with a cardnumber == NULL (and I don't have any idea how it's happen, but I got 1 in my DB :))

(In reply to Jonathan Druart from comment #15)
> But now tests fail if a borrower exists with a cardnumber == NULL (and I
> don't have any idea how it's happen, but I got 1 in my DB :))

Forget that, they pass!

This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Jonathan!

Pushed to 3.10.x, will be in 3.10.10