[m-rev.] for post-commit review: improve test coverage of base_string_to_{int, uint}/3

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Aug 19 13:09:46 AEST 2026



On Wed, 19 Aug 2026 12:54:15 +1000, Julien Fischer <jfischer at opturion.com> wrote:

> Improve test coverage of base_string_to_{int,uint}/3.
> 
> The existing test coverage of base_string_to_int/3 is pretty minimal, and that
> of base_string_to_uint/3 is almost non-existent. Recent changes have
> improved test coverage for the overflow-related aspects of these predicates.
> This diff improves test coverage for the non-overflow-related aspects.

This is a good idea. I have two suggestions.

First, I can't use "diff" to compare the input data sets of the two test cases,
but they seem to be at least similar. I would suggest you replace those two
test cases with a single test case that tests both the signed and unsigned
version of each operation on the same set of test data (the unions of the
current signed and unsigned sets, if they are different), and report just
one result if the signed and unsigned op yield equivalent results.
This should eliminate the need for double maintenance in the future.

Second, I would specify minimum widths for all the inputs in the calls
to io.format. It is easier to check the results if results that should be
identical all start in the same column.

The test inputs seem well chosen, so the diff is otherwise fine.

Zoltan.


More information about the reviews mailing list