https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20478 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Great to see tests for this code, even if done in an unusual way. Some quick remarks: 1. Transactions must be done using txn_begin (also note that you are setting AutoCommit off in the sub and rollback at the end => no sense) 2. $ENV{"OVERRIDE_SYSPREF_dateformat"} = 'metric'; Why not set_preference? Note that for test we usually use mock_preference (from t::lib::Mocks) to avoid to mess with the cache. 3. You could use build_sample_item from bug 21971 to simplify the objects creation (not pushed yet) 4. use Koha::DateUtils instead of DateTime directly 5. You could use File::Slurp::read_file to simplify a bit the code I would have preferred to see the code moved to a module. It would have eased the write of the tests and made the code reusable. Keeping the SO status to get other QA opinions. -- You are receiving this mail because: You are watching all bug changes.