Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve tests by removing EVAL and refactoring #659

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 77 additions & 154 deletions S26-documentation/12-non-breaking-space.t
Original file line number Diff line number Diff line change
@@ -1,163 +1,86 @@
use v6;

use MONKEY-SEE-NO-EVAL;
use Test;

my $code = gen-test();
EVAL $code;

##### subroutines #####
sub gen-test {
my $code = "";
# my $fh = open $f, :w;
#LEAVE try close $fh;

$code ~= q:to/HERE/;
use v6;

# two-column table with various whitespace chars
=begin table
HERE

# non-breaking ws chars
my @nbchars = [
0x00A0, # NO-BREAK SPACE
0x202F, # NARROW NO-BREAK SPACE
0x2060, # WORD JOINER
0xFEFF, # ZERO WIDTH NO-BREAK SPACE
];
# breaking ws chars
my @bchars = [
# don't use the vertical chars for this test which is single-line oriented
#0x000A, # LINE FEED (LF) vertical
#0x000B, # LINE TABULATION vertical
#0x000C, # FORM FEED (FF) vertical
#0x000D, # CARRIAGE RETURN (CR) vertical
#0x2028, # LINE SEPARATOR vertical
#0x2029, # PARAGRAPH SEPARATOR vertical

0x0009, # CHARACTER TABULATION
0x0020, # SPACE
0x1680, # OGHAM SPACE MARK
0x180E, # MONGOLIAN VOWEL SEPARATOR
0x2000, # EN QUAD <= normalized to 0x2002
0x2001, # EM QUAD <= normalized to 0x2003
0x2002, # EN SPACE
0x2003, # EM SPACE
0x2004, # THREE-PER-EM SPACE
0x2005, # FOUR-PER-EM SPACE
0x2006, # SIX-PER-EM SPACE
0x2007, # FIGURE SPACE <= unicode considers this non-breaking, but we won't
0x2008, # PUNCTUATION SPACE
0x2009, # THIN SPACE
0x200A, # HAIR SPACE <= PROBLEM
0x205F, # MEDIUM MATHEMATICAL SPACE
0x3000, # IDEOGRAPHIC SPACE
];

# make one testing row per non-breaking space char
for @nbchars -> $nbc {
# first column
$code ~= "Raku";
$code ~= $nbc.chr;
$code ~= "Language";
for @bchars -> $c {
$code ~= $c.chr;
}
$code ~= "is the best!";

# second column
$code ~= ' | ';
$code ~= "Raku ";
$code ~= $nbc.chr;
$code ~= " Language";
for @bchars -> $c {
$code ~= $c.chr;
}
$code ~= "is the best!\n";
}
$code ~= "=end table\n";

my $n = +@nbchars;
# the planned num of tests is 1 + $n
$code ~= "plan 1 + $n * 2;\n";
$code ~= "my \$n = $n;\n";
$code ~= "my \$SPACE = ' ';\n";

$code ~= q:to/HERE/;
my $r = $=pod[0];
is $r.contents.elems, $n, "table has $n elements (rows)";
HERE

# create separate sub-tests for each non-breaking space char
for 0..^$n -> $i {
$code ~= "\n\{\n";
$code ~= "my \$i = $i;\n";
$code ~= "my \$row = $i + 1;\n";
$code ~= "my \$nbspc = \"{@nbchars[$i].chr}\";\n";

$code ~= q:to/HERE/;
my $res0 = "Raku{$nbspc}Language is the best!";
my $res1 = "Raku {$nbspc} Language is the best!";
# show cell chars separated by pipes
=begin comment
my $c0 = $r.contents[$i][0].comb.join('|');
my $r0 = $res0.comb.join('|');
my $c1 = $r.contents[$i][1].comb.join('|');
my $r1 = $res1.comb.join('|');
=end comment
my $c0 = show-space-chars($r.contents[$i][0]);
my $r0 = show-space-chars($res0);
my $c1 = show-space-chars($r.contents[$i][1]);
my $r1 = show-space-chars($res1);

is $r.contents[$i][0], $res0, "row $row, col 1: '$c0' vs '$r0'";
is $r.contents[$i][1], $res1, "row $row, col 2: '$c1' vs '$r1'";
HERE

$code ~= "}\n";
}

$code ~= q:to/HERE/;
##### some subroutines for the EVALed file #####
sub int2hexstr($int, :$plain) {
# given an int, convert it to a hex string
if !$plain {
return sprintf("0x%04X", $int);
}
else {
return sprintf("%X", $int);
}
}
use lib $?FILE.IO.parent(2).add('packages/Test-Helpers/lib');
use Test::Misc :int2hexstr, :show-space-chars;

plan 9;

# two-column table with various whitespace chars
=begin table
Raku Language  ᠎             is the best! | Raku   Language  ᠎             is the best!
Raku Language  ᠎             is the best! | Raku   Language  ᠎             is the best!
Raku⁠Language  ᠎             is the best! | Raku ⁠ Language  ᠎             is the best!
RakuLanguage  ᠎             is the best! | Raku  Language  ᠎             is the best!
=end table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removal of the pod-generation part is unfortunate. Hard-coding of whitespaces directly into the test code is dangerous because a "malicious" editor could "normalize" some of them. The best solution, to my view, is to use the approach I've shown in #657 (comment) where pod code is EVALed but then the Pod::Block object is pulled from inside of the EVAL scope.


my $n = 4;
my $SPACE = ' ';
my $r = $=pod[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the previous comment in mind, this line would then be replaced with my $r = get-pod; and the rest of the test would remain unchanged. So, we could get the best of two variants: list of chars with codes and names from the initial EVALed version, and easy to work with test code outside of any EVAL.

Copy link
Member Author

@tbrowder tbrowder Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take another look and see what I can come up with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is not in making it smaller but to have only the pod section evaled. The biggest problem with the initial approach is the difficulty to analyze and modify code spread across several ~= statements.

Simply put, from the original test I'm interested in preserving lines 15 to 79.

Copy link
Member Author

@tbrowder tbrowder Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can use your example but I can't see how to generate the code internally in the same comp unit without more EVALing. I can generate it fine from another program.

Suggestions? If you know how to do it please show me a short example.

Copy link
Contributor

@vrurg vrurg Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I got the misunderstanding correctly, I don't mean to get rid of EVAL altogether. I only want to minimize its use. Thus, EVALing the generated code, the way I eval a piece of pod with a primitive sub in my example, is totally ok.

I mean the separation I'm trying to reach is to make the test code, which is to be read and understood by a human, available directly in one clean piece; and only generate and EVAL the pod block as it is just test input data and barely would be modified or analyzed by a person.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I understand what you want, but I don't see how to do it with a single script. Let me try a new tack and see your reaction. I should get it here later today.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me repeat the example code here:

sub get-pod {
    my $pod;
    my sub pull-pod($p) {
        $pod = $p;
    }
    EVAL q:to/TEST/;
        =begin pod
        Pod text goes here...
        =end pod
        pull-pod($=pod[0]);
        TEST
        
    $pod
}
my $pod = get-pod;
isa-ok $pod, Pod::Block, "fine";

This approximately what I'd like to achieve.

Copy link
Member Author

@tbrowder tbrowder Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked again at the situation this morning but I won't start work until you're happy with my proposal below.

First note that I found at least eight test generation programs in roast and that is what I intend to add to the S26 directory. Essentially, I will modify what I did to generate the current test by adding my Raku program and incorporate your excellent method to minimize the EVALed code.

PROPOSAL

  • Create two code templates:

    • test-file.raku.top
    • test-file.raku.bottom
  • Create the Raku test generator to:

    • write the top part to the test file
    • generate and append the EVALed test code to the test file
    • append the bottom part to the test file (I will add some code to all parts as a check on planned tests such how many table rows are expected and how many were actually found)
  • Create a README for the directory to explain the situation

  • Put all three non-test parts in a new tools subdirectory

SUMMARY

I believe my approach helps by:

  • easing creation and debugging of the main top and bottom of the test file
  • clearly show the table generation (the only EVALed code) and characters used
  • explaning the approsch for future devs
  • declutters the main S26 directory by moving the generation code to a subdirectory

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think two extra files and manipulations with them would greatly improve the test.

I probably failed to explain my idea clearly after all. Please, review 4fffc73 and check if it does what it should. I have grouped the tests into subtests. And added two more tests per row (per subtest). So, now, instead of just outputting the converted lines of ws codes it compares them.

is $r.contents.elems, $n, "table has $n elements (rows)";

{
my $i = 0;
my $row = 0 + 1;
my $nbspc = " ";
my $res0 = "Raku{$nbspc}Language is the best!";
my $res1 = "Raku {$nbspc} Language is the best!";
# show cell chars separated by pipes
my $c0 = show-space-chars($r.contents[$i][0]);
my $r0 = show-space-chars($res0);
my $c1 = show-space-chars($r.contents[$i][1]);
my $r1 = show-space-chars($res1);

is $r.contents[$i][0], $res0, "row $row, col 1: '$c0' vs '$r0'";
is $r.contents[$i][1], $res1, "row $row, col 2: '$c1' vs '$r1'";
}

sub show-space-chars($str) {
# given a string with space chars, return a version with the
# hex code shown for them and place a pipe separating all
# chars in the original string
my @c = $str.comb;
my $new-str = '';
for @c -> $c {
$new-str ~= '|' if $new-str;
if $c ~~ /\s/ {
my $int = $c.ord;
my $hex-str = int2hexstr($int);
$new-str ~= $hex-str;
}
else {
=begin comment
$new-str ~= $c;
=end comment
my $int = $c.ord;
my $hex-str = int2hexstr($int, :plain);
$new-str ~= $hex-str;
}
}
return $new-str;
}
HERE
{
my $i = 1;
my $row = 1 + 1;
my $nbspc = " ";
my $res0 = "Raku{$nbspc}Language is the best!";
my $res1 = "Raku {$nbspc} Language is the best!";
# show cell chars separated by pipes
my $c0 = show-space-chars($r.contents[$i][0]);
my $r0 = show-space-chars($res0);
my $c1 = show-space-chars($r.contents[$i][1]);
my $r1 = show-space-chars($res1);

is $r.contents[$i][0], $res0, "row $row, col 1: '$c0' vs '$r0'";
is $r.contents[$i][1], $res1, "row $row, col 2: '$c1' vs '$r1'";
}

$code
{
my $i = 2;
my $row = 2 + 1;
my $nbspc = "⁠";
my $res0 = "Raku{$nbspc}Language is the best!";
my $res1 = "Raku {$nbspc} Language is the best!";
# show cell chars separated by pipes
my $c0 = show-space-chars($r.contents[$i][0]);
my $r0 = show-space-chars($res0);
my $c1 = show-space-chars($r.contents[$i][1]);
my $r1 = show-space-chars($res1);

is $r.contents[$i][0], $res0, "row $row, col 1: '$c0' vs '$r0'";
is $r.contents[$i][1], $res1, "row $row, col 2: '$c1' vs '$r1'";
}

done-testing;
{
my $i = 3;
my $row = 3 + 1;
my $nbspc = "";
my $res0 = "Raku{$nbspc}Language is the best!";
my $res1 = "Raku {$nbspc} Language is the best!";
# show cell chars separated by pipes
my $c0 = show-space-chars($r.contents[$i][0]);
my $r0 = show-space-chars($res0);
my $c1 = show-space-chars($r.contents[$i][1]);
my $r1 = show-space-chars($res1);

is $r.contents[$i][0], $res0, "row $row, col 1: '$c0' vs '$r0'";
is $r.contents[$i][1], $res1, "row $row, col 2: '$c1' vs '$r1'";
}
# vim: expandtab shiftwidth=4
7 changes: 3 additions & 4 deletions S32-str/space-chars.t
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use v6;
use Test;

use lib $?FILE.IO.parent(2).add('packages/Test-Helpers/lib');
use Test::Misc :int2hexstr;

# non-breaking ws chars
my @nbchars = [
0x00A0, # NO-BREAK SPACE
Expand Down Expand Up @@ -79,8 +82,4 @@ for @bchars -> $hexint {
}
}

sub int2hexstr($int) {
return sprintf("0x%04X", $int);
}

# vim: expandtab shiftwidth=4
41 changes: 41 additions & 0 deletions packages/Test-Helpers/lib/Test/Misc.pm6
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use v6;
unit module Test::Misc;

# routines used by:
# S26-documentation/12-non-breaking-space.t
# S32-str/space-chars.t

sub int2hexstr($int, :$plain --> Str) is export(:int2hexstr) {
# Given an int, convert it to a hex string
if !$plain {
return sprintf("0x%04X", $int);
}
else {
return sprintf("%X", $int);
}
}

sub show-space-chars($str --> Str) is export(:show-space-chars) {
# Given a string with space chars, return a version with the hex
# code shown for them and place a pipe separating all chars in the
# original string.
my @c = $str.comb;
my $new-str = '';
for @c -> $c {
$new-str ~= '|' if $new-str;
if $c ~~ /\s/ {
my $int = $c.ord;
my $hex-str = int2hexstr($int);
$new-str ~= $hex-str;
}
else {
=begin comment
$new-str ~= $c;
=end comment
my $int = $c.ord;
my $hex-str = int2hexstr($int, :plain);
$new-str ~= $hex-str;
}
}
return $new-str;
}
4 changes: 3 additions & 1 deletion spectest.data
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ S09-typed-arrays/native.t
S10-packages/basic.t
S10-packages/export.t
S10-packages/joined-namespaces.t
S10-packages/nested-use.t
S10-packages/nested-use.t
S10-packages/precompilation.t # slow
S10-packages/require-and-use.t
S10-packages/scope.t
Expand Down Expand Up @@ -994,6 +994,8 @@ S26-documentation/07c-tables.t
S26-documentation/08-formattingcodes.t
S26-documentation/09-configuration.t
S26-documentation/10-doc-cli.t
S26-documentation/11-non-breaking-space.t
S26-documentation/12-non-breaking-space.t
S26-documentation/14-defn.t
S26-documentation/15-numbered-alias.t
S26-documentation/block-leading-user-format.t
Expand Down