rust-lang / compiler-team

A home for compiler team planning documents, meeting minutes, and other such things.
https://rust-lang.github.io/compiler-team/
Apache License 2.0
387 stars 69 forks source link

Remove the `run-pass-valgrind` test suite #792

Closed jieyouxu closed 1 month ago

jieyouxu commented 1 month ago

Proposal

Remove the run-pass-valgrind test suite. This test suite is not properly hooked up to bootstrap, and run-pass-valgrind tests don't actually get exercised under valgrind. See https://github.com/rust-lang/rust/blob/1b3b8e7b0265162853c650ead09905bc3cdaeae9/src/tools/compiletest/src/runtest/valgrind.rs#L7-L12. Providing valgrind paths was not hooked up to bootstrap and our CI jobs don't run it. opt-dist currently run the run-pass-valgrind tests but because it's not properly hooked up in bootstrap, they are just naive run-pass tests.

Mentors or Reviewers

Bootstrap reviewers (@Kobzol who is already reviewing the https://github.com/rust-lang/rust/pull/131351 PR).

Process

The main points of the Major Change Process are as follows:

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

rustbot commented 1 month ago

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

 @rustbot concern reason-for-concern 
 <description of the concern> 

Concerns can be lifted with:

 @rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler @rust-lang/compiler-contributors

Urgau commented 1 month ago

@rustbot second

Noratrieb commented 1 month ago

i feel like an MCP is overkill for something like this. tests that don't work and don't run are obviously bad! (don't wait for the 10 days, just merge)

apiraino commented 1 month ago

As I said on Zulip, I agree but I felt that an MCP could be useful to let everyone know

apiraino commented 1 month ago

anyway

@rustbot label -final-comment-period +major-change-accepted