https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39405 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Needs Signoff |Failed QA --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Hi Ralph. Good job here. A couple QA remarks: - In the tests file you introduce, you correctly add a subtest that wraps all actions inside a DB transaction. [GOOD] - Even though you wrap the subtest inside a transaction, some data is generated OUTSIDE the transaction i.e. `$branch` and `$category`. Those two should be generated inside the subtest and within the transaction. Otherwise running the test pollutes the DB [BLOCKER] - The hook is added to Koha::Plugin::Test [GOOD] - The fact that your code skips 'undefined result' plugin hook calls is not tested [BLOCKER] - The fact your code will pick the first available value is not tested [BLOCKER] I will submit a patch now, creating two new sample plugins you can use for writing those tests. -- You are receiving this mail because: You are watching all bug changes.