Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest: Support forbid-output in UI tests #134738

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

clubby789
Copy link
Contributor

@clubby789 clubby789 commented Dec 25, 2024

The forbid-output directive is currently only run in incremental tests (although no incremental tests use it). There are some UI tests 'using' it, but it's doing nothing 😄 Let's fix this

Will also PR the dev guide to note this.

dev-guide PR: rust-lang/rustc-dev-guide#2171

@rustbot
Copy link
Collaborator

rustbot commented Dec 25, 2024

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
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

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 25, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 25, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@jieyouxu
Copy link
Member

Wait, so this is like the dual of error-pattern. Can we rename this to forbid-error-pattern maybe?

@jieyouxu jieyouxu assigned jieyouxu and unassigned albertlarsan68 Dec 25, 2024
@jieyouxu
Copy link
Member

Actually, I'll revisit forbid-output in a follow-up because the naming is strange and inconsistent versus error-pattern and normalize-stderr-test and whatever...

@jieyouxu
Copy link
Member

@bors r+ rollup=iffy (who knows if any tests actually should've failed but never did)

@bors
Copy link
Contributor

bors commented Dec 25, 2024

📌 Commit 5a8ecc9 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors 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 Dec 25, 2024
@bors
Copy link
Contributor

bors commented Dec 25, 2024

⌛ Testing commit 5a8ecc9 with merge a0a5c42...

@bors
Copy link
Contributor

bors commented Dec 25, 2024

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing a0a5c42 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 25, 2024
@bors bors merged commit a0a5c42 into rust-lang:master Dec 25, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 25, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a0a5c42): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.4%, 0.4%] 1
Improvements ✅
(primary)
-0.2% [-0.2%, -0.1%] 2
Improvements ✅
(secondary)
-0.3% [-0.4%, -0.2%] 6
All ❌✅ (primary) -0.2% [-0.2%, -0.1%] 2

Max RSS (memory usage)

Results (primary -2.9%, secondary 2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.5% [2.1%, 2.8%] 3
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.9% [-2.9%, -2.9%] 1

Cycles

Results (secondary 4.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.0% [2.4%, 5.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 763.93s -> 763.617s (-0.04%)
Artifact size: 330.53 MiB -> 330.57 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Dec 25, 2024
@lqd
Copy link
Member

lqd commented Dec 25, 2024

Test only change. Gotta be noise.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants