Skip to content

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

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Vastargazing:add-regression-tests-cmp-argument-order
Apr 7, 2026
Merged

coretests: add argument order regression tests for min_by/max_by/minmax_by#154761
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Vastargazing:add-regression-tests-cmp-argument-order

Conversation

@Vastargazing
Copy link
Copy Markdown
Contributor

@Vastargazing Vastargazing commented Apr 3, 2026 •
edited by rustbot
Loading

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 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
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

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

@rustbot

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Copy Markdown
Member

jieyouxu commented Apr 3, 2026

r? libs

@rustbot rustbot assigned jhpratt and unassigned jieyouxu Apr 3, 2026
@jieyouxu jieyouxu mentioned this pull request Apr 3, 2026
Merged
@Vastargazing
coretests: add argument order regression tests for min_by/max_by/minm…
51c4299 
…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.
@Vastargazing Vastargazing force-pushed the add-regression-tests-cmp-argument-order branch from 01a0445 to 51c4299 Compare April 4, 2026 10:35
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 4, 2026
@JonathanBrouwer
Rollup merge of rust-lang#154767 - jieyouxu:lib-tests, r=Mark-Simulacrum
1bc6b6a 
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
rust-timer added a commit that referenced this pull request Apr 5, 2026
@rust-timer
Unrolled build for #154767
070604a 
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
Copy link
Copy Markdown
Member

jhpratt commented Apr 7, 2026

@bors r+ rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 7, 2026

📌 Commit 51c4299 has been approved by jhpratt

It is now in the queue for this repository.

@rust-bors 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 added a commit to jhpratt/rust that referenced this pull request Apr 7, 2026
@jhpratt
Rollup merge of rust-lang#154761 - Vastargazing:add-regression-tests-…
591e3d1 
…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.
@jhpratt jhpratt mentioned this pull request Apr 7, 2026
Closed
rust-bors bot pushed a commit that referenced this pull request Apr 7, 2026
@bors
Auto merge of #154926 - jhpratt:rollup-TfxM9mn, r=jhpratt
a0d655f 
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
Copy link
Copy Markdown
Contributor

JonathanBrouwer commented Apr 7, 2026

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

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors bot pushed a commit that referenced this pull request Apr 7, 2026
@bors
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 added a commit to JonathanBrouwer/rust that referenced this pull request Apr 7, 2026
@JonathanBrouwer
Rollup merge of rust-lang#154761 - Vastargazing:add-regression-tests-…
e28f693 
…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.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 7, 2026
Merged
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 7, 2026

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

rust-bors bot pushed a commit that referenced this pull request Apr 7, 2026
@bors
Auto merge of #154958 - JonathanBrouwer:rollup-PEahluH, r=JonathanBro…
c756124 
…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)
@rust-bors rust-bors bot merged commit 6ee4118 into rust-lang:main Apr 7, 2026
12 checks passed
rust-timer added a commit that referenced this pull request Apr 7, 2026
@rust-timer
Unrolled build for #154761
5f7a370 
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.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 8, 2026
@bors
Auto merge of #154958 - JonathanBrouwer:rollup-PEahluH, r=JonathanBro…
0ce6ad6 
…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)
@Vastargazing Vastargazing deleted the add-regression-tests-cmp-argument-order branch April 8, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jhpratt jhpratt

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Vastargazing @rustbot @jieyouxu @jhpratt @JonathanBrouwer