Bug 37898 - All db dependent tests should run within a transaction
Summary: All db dependent tests should run within a transaction
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-09-11 15:34 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-09-19 11:51 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:
24.11.00
Circulation function:


Attachments
Bug 37898: Wrap Indexer.t inside a transaction (1.81 KB, patch)
2024-09-11 16:07 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37898: Wrap labels tests inside transactions (2.88 KB, patch)
2024-09-11 16:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37898: Wrap Relationships.t inside a transaction (3.55 KB, patch)
2024-09-11 16:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37898: Wrap Patroncards.t inside a transaction (1.89 KB, patch)
2024-09-11 16:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37898: Wrap some ERM api tests inside a transaction (2.78 KB, patch)
2024-09-11 17:50 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37898: Wrap Indexer.t inside a transaction (1.88 KB, patch)
2024-09-17 12:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37898: Wrap labels tests inside transactions (2.94 KB, patch)
2024-09-17 12:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37898: Wrap Relationships.t inside a transaction (3.62 KB, patch)
2024-09-17 12:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37898: Wrap Patroncards.t inside a transaction (1.95 KB, patch)
2024-09-17 12:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37898: Wrap some ERM api tests inside a transaction (2.85 KB, patch)
2024-09-17 12:25 UTC, Martin Renvoize (ashimema)
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 (tcohen) 2024-09-11 15:34:40 UTC
Some tests do not cleanup and leave new data that might interfere with other tests.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-09-11 16:07:40 UTC
This is boring but I'll submit what I've done so far in case someone can continue before I can come back.

I run this to detect candidate tests:

```shell
git grep --files-without-match '\->txn_begin' | grep -e '\.t$' | grep -v -e selenium -e www
```

and this are the ones I haven't checked yet:

```
Record/Record.t
RecordProcessor_EmbedSeeFromHeadings.t
Reports.t
SIP/SIPServer.t
Search.t
Service.t
SuggestionEngine_ExplodedTerms.t
Template/Plugin/KohaDates.t
Templates.t
api/v1/erm_counter_registries.t
api/v1/erm_sushi_services.t
check_kohastructure.t
```
Comment 2 Tomás Cohen Arazi (tcohen) 2024-09-11 16:07:59 UTC
Created attachment 171346 [details] [review]
Bug 37898: Wrap Indexer.t inside a transaction

This tests leave new data on the DB. This patch fixes it.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM biblio \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Biblio count raised
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests pass
=> SUCCESS: Biblio count remains
6. Sign off :-D
Comment 3 Tomás Cohen Arazi (tcohen) 2024-09-11 16:08:01 UTC
Created attachment 171347 [details] [review]
Bug 37898: Wrap labels tests inside transactions

This tests are written in a way that the cleanup is done explicitly. So
I found no evidence of them leaving the database dirty. But it felt
worth wrapping anyway, because any dev adding something could easily
introduce new data inadvertedly.
Comment 4 Tomás Cohen Arazi (tcohen) 2024-09-11 16:08:04 UTC
Created attachment 171348 [details] [review]
Bug 37898: Wrap Relationships.t inside a transaction

This tests leave data on the DB and need to be fixed. This patch does
that.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM borrowers \G" | koha-mysql kohadev
  k$ echo "SELECT COUNT(*) FROM borrower_relationships \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Patron/Relationships.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Numbers don't match the original ones!
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests still pass!
=> SUCCESS: Numbers match!
6. Sign off :-D
Comment 5 Tomás Cohen Arazi (tcohen) 2024-09-11 16:08:10 UTC
Created attachment 171349 [details] [review]
Bug 37898: Wrap Patroncards.t inside a transaction

This tests leave data on the DB and need to be fixed. This patch does
that.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM creator_layouts \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Patroncards.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Numbers don't match the original ones!
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests still pass!
=> SUCCESS: Numbers match!
6. Sign off :-D
Comment 6 Tomás Cohen Arazi (tcohen) 2024-09-11 17:50:55 UTC
Created attachment 171353 [details] [review]
Bug 37898: Wrap some ERM api tests inside a transaction

Trivial ones.
Comment 7 Tomás Cohen Arazi (tcohen) 2024-09-11 17:51:41 UTC
The cases the `git grep` cover are all fixed by this patches.
Comment 8 Martin Renvoize (ashimema) 2024-09-17 12:25:19 UTC
Created attachment 171611 [details] [review]
Bug 37898: Wrap Indexer.t inside a transaction

This tests leave new data on the DB. This patch fixes it.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM biblio \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Biblio count raised
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests pass
=> SUCCESS: Biblio count remains
6. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize (ashimema) 2024-09-17 12:25:21 UTC
Created attachment 171612 [details] [review]
Bug 37898: Wrap labels tests inside transactions

This tests are written in a way that the cleanup is done explicitly. So
I found no evidence of them leaving the database dirty. But it felt
worth wrapping anyway, because any dev adding something could easily
introduce new data inadvertedly.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize (ashimema) 2024-09-17 12:25:23 UTC
Created attachment 171613 [details] [review]
Bug 37898: Wrap Relationships.t inside a transaction

This tests leave data on the DB and need to be fixed. This patch does
that.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM borrowers \G" | koha-mysql kohadev
  k$ echo "SELECT COUNT(*) FROM borrower_relationships \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Patron/Relationships.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Numbers don't match the original ones!
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests still pass!
=> SUCCESS: Numbers match!
6. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize (ashimema) 2024-09-17 12:25:26 UTC
Created attachment 171614 [details] [review]
Bug 37898: Wrap Patroncards.t inside a transaction

This tests leave data on the DB and need to be fixed. This patch does
that.

To test:
1. Run:
   $ ktd --shell
  k$ echo "SELECT COUNT(*) FROM creator_layouts \G" | koha-mysql kohadev
2. Run:
  k$ prove t/db_dependent/Patroncards.t
=> SUCCESS: Tests pass
3. Repeat 1
=> FAIL: Numbers don't match the original ones!
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Tests still pass!
=> SUCCESS: Numbers match!
6. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2024-09-17 12:25:29 UTC
Created attachment 171615 [details] [review]
Bug 37898: Wrap some ERM api tests inside a transaction

Trivial ones.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize (ashimema) 2024-09-17 12:26:01 UTC
Going straight for QA here, thanks for the housekeeping work Tomas.

All tests passing for me and database looking cleaner.

Passing QA
Comment 14 Martin Renvoize (ashimema) 2024-09-19 11:51:33 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant