Improve error handling of Step controller #1862
Conversation
Martchus
commented
Nov 14, 2018
Martchus
commented
- Improve error handling of Step controller and clean up the code a little bit.
- See https://progress.opensuse.org/issues/14904
lib/OpenQA/WebAPI/Controller/Step.pm
Outdated
| my $file = path($job->result_dir(), $module_detail->{text})->open('<:encoding(UTF-8)'); | ||
| my @file_content; | ||
| if (defined $file) { | ||
| @file_content = <$file>; |
There was a problem hiding this comment.
Should be possible to replace this whole block with a slurp.
use Mojo::Util 'decode';
...
my $file_content = decode 'UTF-8', path($job->result_dir(), $module_detail->{text})->slurp;There was a problem hiding this comment.
Note that Mojo::File::open already throws an exception if opening the file failed, so the defined $file check shouldn't be doing anything there.
There was a problem hiding this comment.
That block was already there. It just has been re-intended to keep the code simpler. But I can clean it up in a further commit of course.
There was a problem hiding this comment.
You re-indent it you own it (in git blame). 😉
There was a problem hiding this comment.
We actually have a lot of places where we could use slurp and spurt too... but this ain the place to address that :)
Martchus
force-pushed
the
step_error_handling
branch
from
c225a7f
to
d72d6be
Compare
Codecov Report
@@ Coverage Diff @@
## master #1862 +/- ##
==========================================
+ Coverage 71.73% 90.04% +18.3%
==========================================
Files 127 148 +21
Lines 9433 10348 +915
==========================================
+ Hits 6767 9318 +2551
+ Misses 2666 1030 -1636
Continue to review full report at Codecov.
|
and clean up the code a little bit see https://progress.opensuse.org/issues/14904
|
Seems like using |
as suggested in review
Martchus
force-pushed
the
step_error_handling
branch
from
d72d6be
to
d7ebc2e
Compare
|
In fact the previous code wrongly inserted a white-space and also the test wrongly expected that to be there. So using |
