coretests: add argument order regression tests for min_by/max_by/minmax_by#154761

Vastargazing · 2026-04-03T15:51:57Z

PR #136307 introduced a regression in min_by, max_by, and minmax_by:
the compare closure received arguments as (v2, v1) instead of (v1, v2),
contrary to the documented contract.
Although this was fixed in #139357, no regression tests were added.
This PR adds regression tests for all three functions, verifying that compare
always receives arguments in the documented order (v1, v2).
As a bonus: first coretests coverage for minmax_by.

rustbot · 2026-04-03T15:52:04Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Fallback group: @Mark-Simulacrum, @jieyouxu
@Mark-Simulacrum, @jieyouxu expanded to Mark-Simulacrum, jieyouxu
Random selection from Mark-Simulacrum, jieyouxu

jieyouxu · 2026-04-03T17:19:58Z

r? libs

…ax_by A recent regression swapped the argument order passed to the compare closure in min_by, max_by and minmax_by (compare(&v2, &v1) instead of compare(&v1, &v2)). This was fixed, but no regression test was added. Add tests that record the arguments the compare closure receives and assert they match (v1, v2) — the documented contract.

triagebot: roll library reviewers for `{coretests,alloctests}` Noticed in rust-lang#154761 that we are currently rolling fallback reviewers for `library/{alloctests,coretests}` r? libs

Rollup merge of #154767 - jieyouxu:lib-tests, r=Mark-Simulacrum triagebot: roll library reviewers for `{coretests,alloctests}` Noticed in #154761 that we are currently rolling fallback reviewers for `library/{alloctests,coretests}` r? libs

jhpratt · 2026-04-07T05:39:03Z

@bors r+ rollup

rust-bors · 2026-04-07T05:39:06Z

📌 Commit 51c4299 has been approved by jhpratt

It is now in the queue for this repository.

…cmp-argument-order, r=jhpratt coretests: add argument order regression tests for min_by/max_by/minmax_by PR rust-lang#136307 introduced a regression in min_by, max_by, and minmax_by: the compare closure received arguments as (v2, v1) instead of (v1, v2), contrary to the documented contract. Although this was fixed in rust-lang#139357, no regression tests were added. This PR adds regression tests for all three functions, verifying that compare always receives arguments in the documented order (v1, v2). As a bonus: first coretests coverage for minmax_by.

Rollup of 6 pull requests Successful merges: - #154812 (Update Fira Mono License Information) - #154886 (Stabilize check-cfg suggestions for symbol) - #154889 (Update wasm-component-ld to 0.5.22) - #154761 (coretests: add argument order regression tests for min_by/max_by/minmax_by) - #154825 (constify `Step for NonZero<u*>`) - #154837 (library: std: motor: use OS' process::exit in abort_internal)

JonathanBrouwer · 2026-04-07T14:31:10Z

@bors try jobs=i686-msvc-2
Debugging stuck rollup #154926

…t-order, r=<try> coretests: add argument order regression tests for min_by/max_by/minmax_by try-job: i686-msvc-2

…cmp-argument-order, r=jhpratt coretests: add argument order regression tests for min_by/max_by/minmax_by PR rust-lang#136307 introduced a regression in min_by, max_by, and minmax_by: the compare closure received arguments as (v2, v1) instead of (v1, v2), contrary to the documented contract. Although this was fixed in rust-lang#139357, no regression tests were added. This PR adds regression tests for all three functions, verifying that compare always receives arguments in the documented order (v1, v2). As a bonus: first coretests coverage for minmax_by.

rust-bors · 2026-04-07T16:54:48Z

☀️ Try build successful (CI)
Build commit: dbf9cc1 (dbf9cc12f78354dfa1bac4159fa548123abef55c, parent: 49b6ac01d6f4c3da812039ae846407a20961aa4c)

…uwer Rollup of 22 pull requests Successful merges: - #150965 (Fix no results when searching for == in doc) - #153999 (Remove `TaggedQueryKey::def_kind`) - #154146 (Split out the creation of `Cycle` to a new `process_cycle` function) - #154147 (Do not attempt generating DllImport for extern types) - #154812 (Update Fira Mono License Information) - #154880 (bootstrap: minor improvements to download-rustc) - #154886 (Stabilize check-cfg suggestions for symbol) - #154889 (Update wasm-component-ld to 0.5.22) - #154928 (Fix pin docs) - #154942 (delegation: generate more verbose error delegation) - #153269 (GCI: During reachability analysis don't try to evaluate the initializer of overly generic free const items) - #154506 (Migrate some tests from `tests/ui/issues` to appropriate directories) - #154673 (Use a different name for fast try builds) - #154761 (coretests: add argument order regression tests for min_by/max_by/minmax_by) - #154795 (Add more info about where autodiff can be applied) - #154808 (Post-attribute ports cleanup pt. 1) - #154825 (constify `Step for NonZero<u*>`) - #154837 (library: std: motor: use OS' process::exit in abort_internal) - #154866 (add regression test for #146514) - #154922 (c-b: Export inverse hyperbolic trigonometric functions) - #154931 (delegation(small cleanup): remove not needed PhantomData) - #154950 (library: no `cfg(target_arch)` on scalable intrinsics)

Rollup merge of #154761 - Vastargazing:add-regression-tests-cmp-argument-order, r=jhpratt coretests: add argument order regression tests for min_by/max_by/minmax_by PR #136307 introduced a regression in min_by, max_by, and minmax_by: the compare closure received arguments as (v2, v1) instead of (v1, v2), contrary to the documented contract. Although this was fixed in #139357, no regression tests were added. This PR adds regression tests for all three functions, verifying that compare always receives arguments in the documented order (v1, v2). As a bonus: first coretests coverage for minmax_by.

…uwer Rollup of 22 pull requests Successful merges: - rust-lang/rust#150965 (Fix no results when searching for == in doc) - rust-lang/rust#153999 (Remove `TaggedQueryKey::def_kind`) - rust-lang/rust#154146 (Split out the creation of `Cycle` to a new `process_cycle` function) - rust-lang/rust#154147 (Do not attempt generating DllImport for extern types) - rust-lang/rust#154812 (Update Fira Mono License Information) - rust-lang/rust#154880 (bootstrap: minor improvements to download-rustc) - rust-lang/rust#154886 (Stabilize check-cfg suggestions for symbol) - rust-lang/rust#154889 (Update wasm-component-ld to 0.5.22) - rust-lang/rust#154928 (Fix pin docs) - rust-lang/rust#154942 (delegation: generate more verbose error delegation) - rust-lang/rust#153269 (GCI: During reachability analysis don't try to evaluate the initializer of overly generic free const items) - rust-lang/rust#154506 (Migrate some tests from `tests/ui/issues` to appropriate directories) - rust-lang/rust#154673 (Use a different name for fast try builds) - rust-lang/rust#154761 (coretests: add argument order regression tests for min_by/max_by/minmax_by) - rust-lang/rust#154795 (Add more info about where autodiff can be applied) - rust-lang/rust#154808 (Post-attribute ports cleanup pt. 1) - rust-lang/rust#154825 (constify `Step for NonZero<u*>`) - rust-lang/rust#154837 (library: std: motor: use OS' process::exit in abort_internal) - rust-lang/rust#154866 (add regression test for rust-lang/rust#146514) - rust-lang/rust#154922 (c-b: Export inverse hyperbolic trigonometric functions) - rust-lang/rust#154931 (delegation(small cleanup): remove not needed PhantomData) - rust-lang/rust#154950 (library: no `cfg(target_arch)` on scalable intrinsics)

min/max_by tests: also check result These tests were recently added in rust-lang#154761. IMO there's no reason to ignore the actual result of the function in the test, so let's also assert that this is correct.

…uwer Rollup of 22 pull requests Successful merges: - rust-lang/rust#150965 (Fix no results when searching for == in doc) - rust-lang/rust#153999 (Remove `TaggedQueryKey::def_kind`) - rust-lang/rust#154146 (Split out the creation of `Cycle` to a new `process_cycle` function) - rust-lang/rust#154147 (Do not attempt generating DllImport for extern types) - rust-lang/rust#154812 (Update Fira Mono License Information) - rust-lang/rust#154880 (bootstrap: minor improvements to download-rustc) - rust-lang/rust#154886 (Stabilize check-cfg suggestions for symbol) - rust-lang/rust#154889 (Update wasm-component-ld to 0.5.22) - rust-lang/rust#154928 (Fix pin docs) - rust-lang/rust#154942 (delegation: generate more verbose error delegation) - rust-lang/rust#153269 (GCI: During reachability analysis don't try to evaluate the initializer of overly generic free const items) - rust-lang/rust#154506 (Migrate some tests from `tests/ui/issues` to appropriate directories) - rust-lang/rust#154673 (Use a different name for fast try builds) - rust-lang/rust#154761 (coretests: add argument order regression tests for min_by/max_by/minmax_by) - rust-lang/rust#154795 (Add more info about where autodiff can be applied) - rust-lang/rust#154808 (Post-attribute ports cleanup pt. 1) - rust-lang/rust#154825 (constify `Step for NonZero<u*>`) - rust-lang/rust#154837 (library: std: motor: use OS' process::exit in abort_internal) - rust-lang/rust#154866 (add regression test for rust-lang/rust#146514) - rust-lang/rust#154922 (c-b: Export inverse hyperbolic trigonometric functions) - rust-lang/rust#154931 (delegation(small cleanup): remove not needed PhantomData) - rust-lang/rust#154950 (library: no `cfg(target_arch)` on scalable intrinsics)

Rollup merge of #154995 - RalfJung:minmaxby-tests, r=jhpratt min/max_by tests: also check result These tests were recently added in #154761. IMO there's no reason to ignore the actual result of the function in the test, so let's also assert that this is correct.

rustbot assigned jieyouxu Apr 3, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 3, 2026

This comment has been minimized.

Sign in to view

rustbot assigned jhpratt and unassigned jieyouxu Apr 3, 2026

jieyouxu mentioned this pull request Apr 3, 2026

triagebot: roll library reviewers for {coretests,alloctests} #154767

Merged

Vastargazing force-pushed the add-regression-tests-cmp-argument-order branch from 01a0445 to 51c4299 Compare April 4, 2026 10:35

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2026

jhpratt mentioned this pull request Apr 7, 2026

Rollup of 6 pull requests #154926

Closed

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Apr 7, 2026

Auto merge of #154761 - Vastargazing:add-regression-tests-cmp-argumen…

dbf9cc1

…t-order, r=<try> coretests: add argument order regression tests for min_by/max_by/minmax_by try-job: i686-msvc-2

JonathanBrouwer mentioned this pull request Apr 7, 2026

Rollup of 22 pull requests #154958

Merged

rust-bors bot merged commit 6ee4118 into rust-lang:main Apr 7, 2026
12 checks passed

Vastargazing deleted the add-regression-tests-cmp-argument-order branch April 8, 2026 10:56

RalfJung mentioned this pull request Apr 8, 2026

min/max_by tests: also check result #154995

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

coretests: add argument order regression tests for min_by/max_by/minmax_by#154761