Skip to content

test: fix comparison of integers wider than i128 - #13719

Merged
RenjiSann merged 1 commit into
uutils:mainfrom
dhruv-15-03:test-bigint-compare
Aug 3, 2026
Merged

test: fix comparison of integers wider than i128#13719
RenjiSann merged 1 commit into
uutils:mainfrom
dhruv-15-03:test-bigint-compare

Conversation

@dhruv-15-03

@dhruv-15-03 dhruv-15-03 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #12874

test rejects integers that don't fit in i128:

$ test 1 -eq 16267277278126277227728782172782882627278282882172762677623672762783782
test: invalid integer '16267277278126277227728782172782882627278282882172762677623672762783782'
$ echo $?
2

GNU has no width limit here. All six integer operators are affected.

integers() parsed both operands as i128, so anything wider failed parse().
They're now parsed into a sign plus decimal digits (leading zeros stripped, -0
normalized) and compared on sign, then digit count, then bytewise. No width limit
and no new dependency.

The set of accepted operands is unchanged - 0x10, 1e3, 123.45, 1_0, 5-,
-, +, empty and non-UTF-8 all still fail the same way with exit 2. The digits
are validated here rather than handed to a bignum type because that reject set has
to be preserved either way, and once the validator exists the comparison is short.

test_values_greater_than_i64_allowed was #[ignore]d, but its input fits in
i128 and passes on main today, so the ignore was stale. Re-enabled, with a new
test for i128::MAX + 1 and i128::MIN - 1.

Checked with cargo test -p uu_test and the test_test integration suite, and
diffed against GNU test over 50 cases covering all six operators, large
negatives, sign and leading-zero handling, and the invalid inputs above. Output
matched. The GNU build I had available was 8.32, not 9.x.

integers() parsed both operands with i128, so an operand needing more
digits than that range holds was rejected as an invalid integer with exit
code 2. GNU compares integers of arbitrary width and returns 0 or 1
instead of erroring.

Parse each operand into a sign and its decimal digits and compare those
directly: sign first, then digit count, then bytewise over the remaining
digits. This lifts the width limit from all six integer operators
(-eq, -ne, -lt, -le, -gt, -ge) without adding a dependency, and leaves the
set of accepted operands unchanged.

Fixes uutils#12874

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 2, 2026 18:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Ready to approve

The new comparison logic is self-contained, preserves the documented acceptance/rejection behavior, and is supported by comprehensive new unit and integration tests.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR fixes GNU compatibility in the test utility by removing the effective i128 width limit for integer comparisons, allowing arbitrarily large (decimal) integer operands while preserving the existing “reject set” and error behavior.

Changes:

  • Replace i128 parsing in integer comparisons with a sign+digits representation and lexicographic magnitude comparison (no width limit).
  • Re-enable and expand integration tests covering operands beyond i128, large negatives, mixed signs, and normalization (-0, +0, leading zeros).
  • Add unit tests validating correct ordering and continued rejection of malformed/non-decimal inputs.
File summaries
File Description
tests/by-util/test_test.rs Re-enables stale ignored test and adds integration coverage for very large integer comparisons and malformed operand handling.
src/uu/test/src/test.rs Implements width-unlimited integer comparison by parsing operands into sign+digits and comparing by sign/length/lexicographic order; adds unit tests.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@codspeed-hq

codspeed-hq Bot commented Aug 3, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 22.61%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 344 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation df_with_path 699.9 µs 570.8 µs +22.61%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing dhruv-15-03:test-bigint-compare (a31150b) with main (b13ee7a)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tty-eof (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)

@RenjiSann

Copy link
Copy Markdown
Collaborator

PR message is much too verbose, please write it by hand instead of proxying the burden of understanding both your patch and AI slop to maintainers.

@dhruv-15-03

Copy link
Copy Markdown
Contributor Author

Fair point - I've cut it down to the problem, the cause and the fix.

@RenjiSann

Copy link
Copy Markdown
Collaborator

Thank you !

@RenjiSann
RenjiSann merged commit fe9420b into uutils:main Aug 3, 2026
162 of 163 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gnucompability(test): large numbers rejected while gnu accepted

3 participants