Tidied up unit test t/80_diag.t script.

[manwar]

Hi @makamaka

Please review the PR. This would help us to get more detailed error in case like below:

  Cannot read error messages from PP
  BEGIN failed--compilation aborted at t/80_diag.t line 21.
  # Looks like your test exited with 2 just after 1.
  t/80_diag.t .............. 
  Dubious, test returned 2 (wstat 512, 0x200)
  Failed 285/286 subtests

https://www.cpantesters.org/cpan/report/fd81c3da-6f9c-11e8-aef1-3a512fb80328

Many Thanks. Best Regards, Mohammad S Anwar

[charsbar]

Hi @manwar , thanks for the PR,

Most of the tests under t/, including t/80_diag.t, are copied from Text::CSV_XS with some modification (like s/XS/PP/ and such) using author/bin/sync.pl ( https://github.com/makamaka/Text-CSV/blob/master/author/bin/sync.pl ) to make sure both Text::CSV_XS and Text::CSV_PP pass the same tests.

So could you send the same kind of PR to https://github.com/Tux/Text-CSV_XS instead of here? If it is merged, eventually the change will be in (when I have some time to sync).

[Tux]

@manwar I am - now that I dropped support for overly old perl versions - very much in favor of using lexical file handles, but what your PR offers is too much noise and not matching the requirements in Text::CSV_XS' CONTRIBUTING.md: very wrong indent to start with. Feels like unneeded overengineering. I could live with:

diff --git a/t/80_diag.t b/t/80_diag.t
index d87497c..33d0aa9 100644
--- a/t/80_diag.t
+++ b/t/80_diag.t
@@ -13,10 +13,11 @@ BEGIN {
     plan skip_all => "Cannot load Text::CSV_XS" if $@;
     require "./t/util.pl";

-    open XS, "<", "CSV_XS.xs" or die "Cannot read error messages from XS\n";
-    while (<XS>) {
+    open my $fh, "<", "CSV_XS.xs" or die "Cannot read error messages from XS\n";
+    while (<$fh>) {
        m/^    \{ ([0-9]{4}), "([^"]+)"\s+\}/ and $err{$1} = $2;
        }
+    close $fh;
     }

 $| = 1;

If you want your credits, you try again with the above and include similar changes to t/65_allow.t, t/90_csv.t and t/91_csv_cb.t

Be sure to test it under perl-5.6.1, which is the oldest perl still under support

[manwar]

Hi @Tux

Thanks for your feedback, much appreciated. I will try to do as you suggested. Before that I will try to get a system with Perl 5.6.1 to test my changes against.

Best Regards, Mohammad S Anwar

[manwar]

Hi @Tux

I have updated the test scripts as per your suggestions (except t/90_csv.t). Please review the changes.

Many Thanks. Best Regards, Mohammad S Anwar

[Tux]

@manwar I like all but line 20 of 80_diag.t where you change the tab to spaces. Make it a PR and I'll take it

[manwar]

Hi @Tux

Please review it now.

Many Thanks. Best Regards, Mohammad S Anwar

[Tux]

Can you make it a PR to Text::CSV_XS?

[manwar]

Sure :-)

[Tux]

Just to be clear about spaces vs. tabs: I really don't care at all what the policy of the author of the module(s) is. My personal pref is mixed tabs and spaces, but I know a lot of coders hate that. What I however do care about is consistency, and all the other code uses tabs

[charsbar]

Shipped Text::CSV 1.96, which should have imported the changes you made to Text::CSV_XS. Thanks.