koha_1 | 2012, EOF - End of data in parsing input stream, 0 at /kohadevbox/koha/Koha/BackgroundJob/ImportKBARTFile.pm line 247, <$fh> line 3. koha_1 | 2012, EOF - End of data in parsing input stream, 0 at /kohadevbox/koha/Koha/BackgroundJob/ImportKBARTFile.pm line 247, <$fh> line 3. koha_1 | 2012, EOF - End of data in parsing input stream, 0 at /kohadevbox/koha/Koha/BackgroundJob/ImportKBARTFile.pm line 247, <$fh> line 3. koha_1 | 2012, EOF - End of data in parsing input stream, 0 at /kohadevbox/koha/Koha/BackgroundJob/ImportKBARTFile.pm line 247, <$fh> line 3. koha_1 | [15:09:28] t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t ......................... ok 3354 ms ( 0.01 usr 0.00 sys + 2.84 cusr 0.32 csys = 3.17 CPU)
Created attachment 167078 [details] [review] Bug 36935: Remove warning if file parsing is successful This patch removes the logged warning if a file parse is successful. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t 2) The output will have a logged message - 'EOF - End of data in parsing input stream' 3) Apply patch 4) Repeat step 1 5) The warning will be gone
errno 2012 means successful?
Apparently so! https://metacpan.org/pod/Text::CSV_XS#DIAGNOSTICS
Created attachment 167080 [details] [review] Bug 36935: Remove warning if file parsing is successful This patch removes the logged warning if a file parse is successful. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t 2) The output will have a logged message - 'EOF - End of data in parsing input stream' 3) Apply patch 4) Repeat step 1 5) The warning will be gone Signed-off-by: David Nind <david@davidnind.com>
(In reply to Matt Blenkinsop from comment #3) > Apparently so! > https://metacpan.org/pod/Text::CSV_XS#DIAGNOSTICS I am not sure, this is not clear for me.
use Text::CSV_XS; open(my $fh, "<", '/tmp/file') or die $!; my $csv = Text::CSV_XS->new( { sep_char => ',', binary => 1, allow_loose_quotes => 1 } ); my @lines = @{$csv->getline_all($fh)}; close($fh); my ( $cde, $str, $pos ) = $csv->error_diag(); my $error = $cde ? "$cde, $str, $pos" : ""; use Data::Printer colored => 1; say p @lines; /tmp/file contains one,two,three 1,2,3 4,5,6 7,8,9 run the script and get no error
open my $fh, "<", \$file_content or die; This must die with the reason! The bare minimum is 'die $!'
Or add a newline to the input file or so ?
Created attachment 167088 [details] [review] Bug 36935: Remove warning if file parsing is successful This patch removes the logged warning if a file parse is successful. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t 2) The output will have a logged message - 'EOF - End of data in parsing input stream' 3) Apply patch 4) Repeat step 1 5) The warning will be gone
Created attachment 167089 [details] [review] Bug 36935: Add a 'die' message to file parsing
Is this error handling covered by a test?
Not currently, I'll add one. Patch incoming
If I force the error handling to throw an error in a unit test we will get a warning in the output which is what we're trying to avoid. Now that we're identifying the end of file we could throw a Koha::Exception rather than a warning?
Use Test::Warn (warning_is for instance) if you want to test that a warning will be displayed in the log. An exception would be better ofc.
Created attachment 167099 [details] [review] Bug 36935: Add unit test for error handling prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t
Bug 36942 raised for throwing an exception, will try and have a look at that when I have some time
Created attachment 167130 [details] [review] Bug 36935: Remove warning if file parsing is successful This patch removes the logged warning if a file parse is successful. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t 2) The output will have a logged message - 'EOF - End of data in parsing input stream' 3) Apply patch 4) Repeat step 1 5) The warning will be gone Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167131 [details] [review] Bug 36935: Add a 'die' message to file parsing Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167132 [details] [review] Bug 36935: Add unit test for error handling prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 168326 [details] [review] Bug 36935: Remove warning if file parsing is successful This patch removes the logged warning if a file parse is successful. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t 2) The output will have a logged message - 'EOF - End of data in parsing input stream' 3) Apply patch 4) Repeat step 1 5) The warning will be gone Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 168327 [details] [review] Bug 36935: Add a 'die' message to file parsing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 168328 [details] [review] Bug 36935: Add unit test for error handling prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Matt, always good to make the tests a little cleaner :)
Pushed for 24.11! Well done everyone, thank you!