http://qs321.pair.com?node_id=11122156


in reply to RFC: How did I do writing my first test?

I'm not convinced that testing of a final new line adds any value, especially given that your code looks agnostic to its presence/absence (from both the code writer and code users point of view).

Putting the tests in subroutines (one for each group of tests) would make the code more skimmable, and save you from needing to use arcane names like ($n_fh, $n_file) i.e. having that in a sub means they would only exist in one place and can have more 'normal' names like in the encoding test case.

As for the code itself, 'fancy_open' is not an accurate name because it is doing reading and closing as well as opening. In Perl parlance, this is called "slurping", so I'd suggest "fancy_slurp".

Slurping into an array is fine for small files, but will become inefficient as the input files get larger, so you may want to consider providing an iterator as well.

Why keep the POD in a separate file? POD is designed to be embeddable in the module and e.g. to veiw it you could just do:

perldoc My::Module

Replies are listed 'Best First'.
Re^2: RFC: How did I do writing my first test?
by Lady_Aleena (Priest) on Sep 24, 2020 at 16:28 UTC

    Testing files that end with newlines was suggested here. I would remove those tests and happily only have one temp file for all the tests.

    I am having a hard time visualizing the tests in subroutines. I am also willing to expand the variable names. I was being a bit lazy when I wrote them. ($n_fh, $n_file) becomes ($newline_fh, $newline_file); ($no_n_fh, $no_n_file) becomes ($no_newline_fh, $no_newline_file). However, if the tests for files ending with a newline are removed, I will just use the ($fh, $fn) from line 84. I will push the test with the expanded variable names shortly after posting this.

    I will think about changing the name, however, I do not like the word slurp.

    Putting my POD in separate files came about several weeks ago. I was writing up usage of a site specific module for myself, finally, in a separate file. I was going to save it as text. All it needed was a little formatting, so I added the POD formatting to it. I saved it as .pod, committed it, and pushed it to GitHub. Out of curiosity, I looked at it on GitHub and was happy to see the POD was rendered into HTML. Since it is easier to read when rendered as HTML, I decided to do it for all my modules. The only time I ever read a Perl document with perldoc is when I am checking to see what my POD looks like. Otherwise, I read the Perl and module documentation in a browser (perldoc.perl.org or metacpan.org). So, since GitHub will render .pod as HTML, making it easier to read, I thought to myself, "Why not?". Also, perldoc My::Module still works whether the POD is embedded or separate. I checked.

    My OS is Debian 10 (Buster); my perl versions are 5.28.1 local and 5.16.3 or 5.30.0 on web host depending on the shebang.

    No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
    Lady Aleena
      Using subroutines could be something as simple as
      sub test_encodings { my ($fh, $fn) = tempfile(); $fh->print($file_string); $fh->close(); is_deeply( [ Fancy::Open::fancy_open($fn) ], [ map encode('utf8', $_), @wanted_array ], "testing an array opened with no encoding" ); my @encodings = qw(UTF-8 ascii cp1252 iso-8859-1); for my $encoding (@encodings) { is_deeply( [ Fancy::Open::fancy_open($fn, { 'encoding' => $encoding }) ], [ map encode($encoding, $_), @wanted_array ], "testing an array opened with $encoding encoding" ); } } sub test_with_file_that_ends_with_a_newline { my ($fh, $fn) = tempfile(); $fh->print("$file_string\n"); $fh->close(); is_deeply( [ Fancy::Open::fancy_open($fn) ], [ @wanted_array ], "testing a plain array with file that ends with a newline" ); is_deeply( [ Fancy::Open::fancy_open($fn, { 'before' => 'solid ' }) ], [ @solid_array ], "testing an array with before option with file that ends with a +newline" ); is_deeply( [ Fancy::Open::fancy_open($fn, { 'after' => ' bead' }) ], [ @bead_array ], "testing an array with after option with file that ends with a n +ewline" ); is_deeply( [ Fancy::Open::fancy_open($fn, { 'before' => 'solid ', 'after' = +> ' bead' }) ], [ @solid_bead_array ], "testing an array with before and after options with file that e +nds with a newline" ); }
      As bonuses, the file name/handle variable names are uniform, and the section comments are made redundant by the sub names.

      On second thought, "slurp" (in Perl land) means reading a file into a scalar, whereas you're reading into an array, so "fancy_read_lines" is still more accurate than the current name.

      I didn't realise GitHub could automatically render .pod files - that is cool.

Re^2: RFC: How did I do writing my first test?
by jcb (Parson) on Sep 24, 2020 at 22:30 UTC

    Testing the final newline presence/absence is still beneficial to ensure that the code stays agnostic to it. Further, if the code handles those cases differently, testing that processing is needed for code coverage.

      I'm not sure ... (on closer inspection of what it does) that seems like basically retesting Perl's readline function, which is already tested elsewhere.

        I would offer that such testing is still reasonable, to ensure that the behavior does not change if/when some future version implements more complex wrapping around readline.

        If the high-level specification says that the code behaves "like so" with/without a final newline, the testsuite should verify that, even if it is effectively verifying part of perl itself in the current implementation. If nothing else, this will blow up if some future perl changes that behavior, alerting to the need to either add code to maintain the previous behavior or revise the specification.

Re^2: RFC: How did I do writing my first test?
by Lady_Aleena (Priest) on Sep 26, 2020 at 01:59 UTC

    After all of the talk about final newlines, and a problem just hit me in the head. I did not take into consideration blank lines in files. What if a file has a blank line or two in the middle? If there is a blank line it currently, the before and after still get put on the list without any other value.

    test.txt (file)
    red orange yellow spring green teal cyan azure blue violet magenta pink white black gray
    Code
    perl -E ' use lib "mods/lib"; use Fancy::Open qw(fancy_open); my @array=fancy_open("test.txt", { before => "solid ", after => " bead +" }); say $_ for @array; '
    Result
    solid red bead solid orange bead solid yellow bead solid spring bead solid green bead solid teal bead solid cyan bead solid azure bead solid bead solid blue bead solid violet bead solid magenta bead solid pink bead solid white bead solid black bead solid gray bead

    So, I am now three steps back from being ready to package this module. *sighs*

    I didn't feel like fighting with splice right now to add an empty line in the test script.

    My OS is Debian 10 (Buster); my perl versions are 5.28.1 local and 5.16.3 or 5.30.0 on web host depending on the shebang.

    No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
    Lady Aleena

      This is why you should write full-coverage testsuites — in the course of testing every path, you are likely to find edge cases that you had not recognized before.

      For this particular problem, the easiest solution, assuming that you chomp each input line is to put the code that adds before and after strings in an unless (m/^$/) block. (If you do not chomp the input, change the regex match to m[^$/$], quickly tested here with perl -e 'while (<>) { print "empty\n" if m[^$/$] }' and typing some sample input.)

        "This is why you should write full-coverage testsuites — in the course of testing every path, you are likely to find edge cases that you had not recognized before."

        An API (or any code) should have tests written before, during and after completion. Especially after the documentation is completed, to verify.

        You're right about the edge cases. They get found in testing, and then become tests themselves.

        If one doesn't find issues when running their test suites against their code after significant updates, the test suite is insufficient.

      What does the documentation say about how it handles blank lines? :)

      i.e. this may well be the correct behaviour (but you'd need to think about and document that).