|
Description
Jonathan Druart
2024-05-23 07:38:31 UTC
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! |