HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Perl module validate quoted string literal, test with Test::LectroTest

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
validateperlwithmoduleliterallectrotestquotedteststring

Problem

I guess for some motivation: I ripped this out of a larger library I'm half-working on to parse S-expressions and cleaned it up a bit.

I'm trying to write a simple function that tests whether a string is a valid quoted string literal. Using Test::LectroTest to compare a fast version of a function using a regex against a slow version that uses a Perl for loop.

I have a couple complaints / style questions right off the bat:

  • Is the lib/QuotedString.pm the right place for the accept_string_slow function to live? I don't intend for people to actually use this function, it's just there to support the tests. Is there a way to hide it better?



  • I can't actually figure out how to influence how LectroTest generates strings. Since it picks characters uniformly at random, I decided the clearest thing to do would be to accept the string it produced but add quotations marks to exercise different edge cases.



  • The general style is probably bad. I don't write Perl professionally.



Directory layout:

.
|-- lib
|   `-- QuotedString.pm
`-- t
    `-- test_accept_string_generative.t


Here is lib/QuotedString.pm.

```
package QuotedString;

BEGIN {
require Exporter;
@ISA = qw[Exporter];
@EXPORT_OK = qw[
accept_string_regex
accept_string_slow
];
};

use strict;
use warnings;

use Readonly;

Readonly my $string_pattern => qr/"*["]/;

sub accept_string_regex {
my $string = shift;
($string =~ m/\A$string_pattern\z/) ? 1 : 0;
}

sub accept_string_slow {
my $string = shift;
return 0 if length($string) < 2;
return 0 unless $string =~ /\A"/;
return 0 unless $string =~ /"\z/;

my $index = 1;
my $final_checked_char = length($string) - 2;
while ($index <= $final_checked_char) {
my $char = substr($string, $index, 1);
# if we encounter an unescaped ", we fail
return 0 if $char eq '"';
# \ consumes the next character fail if there is no next character bef

Solution

Location of your helper function

If you place the accept_string_slow function inside of your lib/QuotedString.pm, it's visible to everyone. That means that someone will use it.

There is no concept of private in Perl. We have a convention to prefix things with an underscore _ so people know it's something they should not be messing with. But that does not stop them.

If this function is really only for testing, then you have a few options. It looks like it's your expected output, which because the input is random, you built into a function. That's a good approach.

The easiest and in my opinion most appropriate way is to put it right into your test .t file. It's part of that very specific test and is not used anywhere else. Just place it under the use QuotedString and remove the name from the use line.

use QuotedString qw[accept_string_regex];

sub accept_string_slow {
    my $string = shift;
    return 0 if length($string) < 2;
    return 0 unless $string =~ /\A"/;
    return 0 unless $string =~ /"\z/;

    # ...
}

Property {
    ##[ x <- String ]##
    accept_string_slow($x) eq accept_string_regex($x);
};


And that's it. Your calls to the two functions inside of the Property now still work, because all that the use does is import the function from the QuotedString package into the local package (which is main). You don't need to worry about how that works.

I would also recommend renaming the function to something that tells the maintainer (which might well be future-you) what the heck is going on here. Maybe go with sub expected_output or similar. Since it's now contained in this test you don't need to adhere to any naming conventions that the project might have. It's your code, make it as readable as possible.

Alternative ways would be to either monkeypatch the function into the QuotedString package, which makes sense if you want to overwrite a certain behavior, or to use a separate library and put it e.g. in t/lib/Helper/QuotedString.pm, which makes sense for larger changes or if you need an entire class to help you out during your tests.

Influencing LectroTest

This is a bit more suited for Stack Overflow, but here's a run-down anyway.

You should first read the docs of Test::LectroTest::Generator. That module gives you everything you need. You can use the Paste combinator with the Unit generator and the String generator. Here's an example that just outputs 20 combinations.

use strict;
use warnings;
use feature 'say';
use Test::LectroTest trials => 20;
use Test::LectroTest::Generator ':all';

Property {
    ##[ x "A-Z", length=>[1,10] ) ) ]##
    say "x = $x";
};


Unit gives you a fixed value that never changes. Kind of like a constant. String you already know, but it seems we need to be more specific if it's used with Paste. I have not figured out why yet. The arguments charset and length should be self-explanatory. You can use single characters, like abc to allow a, b, and c, character ranges like A-F to create ABCDEF automatically. Those can also be combined as in AB-EFg. The length here means between one and ten.

Paste concatenates them. Actually it's more like a join, but if you omit the last argument glue, it uses an empty string and thus it's really a concat. (Beware, there is a Concat that does something else!)

1..1
x = "YSJAQXSG
x = "WWU
x = "IVKJGLWVV
x = "TWQHU
x = "DVXJDTRTX
x = "CC
x = "SULEP
x = "NFBTM
x = "UR
x = "TKS
x = "IUXIMDLUWX
x = "WHYLRV
x = "CWIQLLGO
x = "N
x = "J
x = "FZOLCBCUZX
x = "FN
x = "ZVAQZJSN
x = "TIYUXJV
x = "SVE
ok 1 - 'Unnamed Test::LectroTest::Property' (20 attempts)


This seems to work. You can also add more than one row of input definitions. That way, you can combine several tests into one Property. This is explained in Test::LectroTest::Property.

Property {
    ##[ x "A-Z", length=>[1,10] ) ) ],
    [ x "A-Z", length=>[1,10] ), Unit('"') ) ]##
     say "x = $x";
};


I've reduced the trials to 5 to make it easier to read. It will run each line of definitions 5 times.

1..1
x = "CIWNIGSNL
x = "YI
x = "GPCAEZYO
x = "EPLKF
x = "ABUD
x = HS"
x = JGW"
x = QLZJUAZ"
x = NUQEKGL"
x = RSGSVOAXAQ"
ok 1 - 'Unnamed Test::LectroTest::Property' (10 attempts)


This could now also be used to do the "foo" check where the enclosing quotes are intact.

If you want to instead of spelling all of this out, pre-build a generator and use the variable in your definitions, you can do the following. The Test::LectroTest::Generator documentation does this, but fails to explain how to use it with the comment-style syntax.

my $gen = Paste( Unit('"'), String( charset => "A-Z", length => [ 1, 10 ] ) );

Property {
    ##[ x <- $gen ]##
    say $x;
};


To go a bit further, it would also be possible to build a definition that sometimes injects " or \\ or \\" with reduced likelihood.

```
my $overkill = Paste(
(
Frequency(
[ 95, Char( charset => "

Code Snippets

use QuotedString qw[accept_string_regex];

sub accept_string_slow {
    my $string = shift;
    return 0 if length($string) < 2;
    return 0 unless $string =~ /\A"/;
    return 0 unless $string =~ /"\z/;

    # ...
}

Property {
    ##[ x <- String ]##
    accept_string_slow($x) eq accept_string_regex($x);
};
use strict;
use warnings;
use feature 'say';
use Test::LectroTest trials => 20;
use Test::LectroTest::Generator ':all';

Property {
    ##[ x <- Paste( Unit('"'), String( charset=>"A-Z", length=>[1,10] ) ) ]##
    say "x = $x";
};
1..1
x = "YSJAQXSG
x = "WWU
x = "IVKJGLWVV
x = "TWQHU
x = "DVXJDTRTX
x = "CC
x = "SULEP
x = "NFBTM
x = "UR
x = "TKS
x = "IUXIMDLUWX
x = "WHYLRV
x = "CWIQLLGO
x = "N
x = "J
x = "FZOLCBCUZX
x = "FN
x = "ZVAQZJSN
x = "TIYUXJV
x = "SVE
ok 1 - 'Unnamed Test::LectroTest::Property' (20 attempts)
Property {
    ##[ x <- Paste( Unit('"'), String( charset=>"A-Z", length=>[1,10] ) ) ],
    [ x <- Paste( String( charset=>"A-Z", length=>[1,10] ), Unit('"') ) ]##
     say "x = $x";
};
1..1
x = "CIWNIGSNL
x = "YI
x = "GPCAEZYO
x = "EPLKF
x = "ABUD
x = HS"
x = JGW"
x = QLZJUAZ"
x = NUQEKGL"
x = RSGSVOAXAQ"
ok 1 - 'Unnamed Test::LectroTest::Property' (10 attempts)

Context

StackExchange Code Review Q#150663, answer score: 3

Revisions (0)

No revisions yet.