When a method is not allowed yet (not covered by tests), developers get an error the is not really useful: "The method XXX is not covered by tests!" It would be more useful to have a stack trace in order to know where this error come from.
Created attachment 75319 [details] [review] Bug 20767: Display a stack trace when a method is not covered by tests When a method is not allowed yet (not covered by tests), developers get an error the is not really useful: "The method XXX is not covered by tests!" It would be more useful to have a stack trace in order to know where this error come from. Test plan: Modify some code to display this stack trace and confirm it makes sense to have it, i.e. it will make debug easier
(In reply to Jonathan Druart from comment #0) > When a method is not allowed yet (not covered by tests) Is this about automated tests? Then what does this has to do when executing the method? Is this a way to make people to write tests? It seems that there can be other causes to this error, is that correct? I also heard that this had to do with the mapping with de DB columns being done or not. So, I'm confused ^^"
(In reply to Victor Grousset/tuxayo from comment #2) > (In reply to Jonathan Druart from comment #0) > > When a method is not allowed yet (not covered by tests) > > Is this about automated tests? Then what does this has to do when executing > the method? Is this a way to make people to write tests? > > It seems that there can be other causes to this error, is that correct? > > I also heard that this had to do with the mapping with de DB columns being > done or not. So, I'm confused ^^" We can get this error when we try to use a new column and Koha::Schema was not updated. So, the message should probably be "This method does not exist" rather than "not covered by tests".
I have a (good ?) example on my test instance : when I try to use the new syspref to mark an item lost and notify patron from holds to pull list, I get this software error : The method homebranch is not covered by tests!" Applying this patch, I get this one : The method Koha::Patron->homebranch is not covered by tests! Trace begun at /var/repositories/koha/Koha/Object.pm line 398 Koha::Object::AUTOLOAD('Koha::Patron=HASH(0x8d10e08)') called at /var/repositories/koha/ circ/pendingreserves.pl line 71 Is it the behaviour you were looking for ? As I don't know what you expect I prefer not signing it.
Created attachment 75547 [details] [review] Bug 20767: Display a stack trace when a method is not covered by tests When a method is not allowed yet (not covered by tests), developers get an error the is not really useful: "The method XXX is not covered by tests!" It would be more useful to have a stack trace in order to know where this error come from. Test plan: Modify some code to display this stack trace and confirm it makes sense to have it, i.e. it will make debug easier Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 75552 [details] [review] Bug 20767: Display a stack trace when a method is not covered by tests When a method is not allowed yet (not covered by tests), developers get an error the is not really useful: "The method XXX is not covered by tests!" It would be more useful to have a stack trace in order to know where this error come from. Test plan: Modify some code to display this stack trace and confirm it makes sense to have it, i.e. it will make debug easier Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Julian Maurice from comment #3) > So, the message should probably be "This method does not exist" rather than > "not covered by tests". Any opinion on that ? Should I open a new bug ?
(In reply to Julian Maurice from comment #7) > (In reply to Julian Maurice from comment #3) > > So, the message should probably be "This method does not exist" rather than > > "not covered by tests". > > Any opinion on that ? Should I open a new bug ? This method is not supported ? If you only replace a few words, please add a QA follow-up?
(In reply to Julian Maurice from comment #7) > (In reply to Julian Maurice from comment #3) > > So, the message should probably be "This method does not exist" rather than > > "not covered by tests". > > Any opinion on that ? Should I open a new bug ? I agree. File a new one!
Pushed to master for 18.11. Awesome work all, thank you.
We need it to be backported to help debug!
This patch has been pushed to 18.05.x and will be in 18.05.01 Go forth and debug all the things :)
Pushed to 17.11.x for 17.11.07
I choose not to backport to 17.05.x, I focus on major and blocker