[m-rev.] for review: make test hard_coded/string_class test more comprehensive
Zoltan Somogyi
zoltan.somogyi at runbox.com
Mon Sep 21 00:38:26 AEST 2026
On Sun, 20 Sep 2026 20:08:30 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> Make the hard_coded/string_class test more comprehensive.
For total rewrites like this, sending just the new version is easier
on reviewers than the diff.
> - Test each test predicate on every test string.
> - Include is_empty/1 is_all_alnum/1 in the predicates to test.
> - Add some tests the boundaries of the ASCII range.
> - Add some tests involving ill-formed strings.
>
> Generate the output as a table for ease of comparison.
That's a set of big steps forward.
> +empty = is_all_empty alpha_ = is_all_alpha_or_underscore
> +alpha = is_all_alpha alnum_ = is_all_alnum_or_underscore
> +alnum = is_all_alnum match = all_match(ascii_alpha)
> +digit = is_all_digits nonasc = all_match(nonascii)
These should have shorter abbreviations (such as AN_ or even _N
for is_all_alnum_or_underscore) to allow four or even three chars
per column. As is, each row appears split across two lines, at least
in both my email readers.
> --- a/tests/hard_coded/string_class.m
> +++ b/tests/hard_coded/string_class.m
> @@ -1,6 +1,20 @@
> %---------------------------------------------------------------------------%
> % vim: ts=4 sw=4 et ft=mercury
> %---------------------------------------------------------------------------%
> +%
> +% Test the string predicates that test if every code point in a string
> +% belongs to some class of characters.
s/if/whether/
> +% Each row of the output gives the result of every such predicate on one
> +% test string.
> +%
> +% The .exp file is for backends using UTF-8 string encoding.
> +% The .exp2 file is for backends using UTF-16 string encoding.
s/UTF-X/UTF-X as the/
> +% The two outputs differ only in the rows for ill-formed strings: on UTF-8
> +% backends an ill-formed code unit is a lone byte, while on UTF-16 backends
> +% it is an unpaired surrogate code point.
Add a comma after both occurrences of "backends".
> +test_string(Str, !IO) :-
> + write_row(describe(Str),
> + tf(is_empty(Str)),
> + tf(is_all_alpha(Str)),
> + tf(is_all_alnum(Str)),
> + tf(is_all_alpha_or_underscore(Str)),
> + tf(is_all_alnum_or_underscore(Str)),
> + tf(is_all_digits(Str)),
> + tf(all_match(ascii_alpha, Str)),
> + tf(all_match(nonascii, Str)),
> + !IO).
> +
> +:- pred write_row(string::in, string::in, string::in, string::in, string::in,
> + string::in, string::in, string::in, string::in, io::di, io::uo) is det.
> +
> +write_row(Str, Empty, Alpha, Alnum, AlphaU, AlnumU, Digit, Match, NonAscii,
> + !IO) :-
The first argument here is not really Str, it is describe(Str). I would
therefore rename it as Desc.
I would put the table of test strings at the end of the module.
Other than that, the diff is fine.
Zoltan.
More information about the reviews
mailing list