Description
Tomás Cohen Arazi (tcohen)
2024-09-11 15:34: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 ``` 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 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. 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 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 Created attachment 171353 [details] [review] Bug 37898: Wrap some ERM api tests inside a transaction Trivial ones. The cases the `git grep` cover are all fixed by this patches. 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> 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> 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> 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> 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> Going straight for QA here, thanks for the housekeeping work Tomas. All tests passing for me and database looking cleaner. Passing QA Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant |