[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