Enable `--check-cfg` by default in UI tests by Urgau · Pull Request #124345 · rust-lang/rust

Urgau · 2024-04-24T20:17:24Z

This PR enables-by-default --check-cfg in UI tests, now that it has become stable.

To do so this PR does 2 main things:

it introduce the no-auto-check-cfg directive to compiletest, to prevent any --check-cfg args (only to be used for --check-cfg tests)
it updates the remaining¹ UI tests by either:
- allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
- give the appropriate check-cfg args
- or expect the lint, when it useful

I highly recommend reviewing this PR commit-by-commit.

r? @jieyouxu

some preparation work was done in Do some preparation work for compiletest check-cfg #123577 Further cleanup cfgs in the UI test suite #123702 ↩

ghost · 2024-04-24T20:17:35Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

ghost

The code changes generally look good to me. Just one question about revision normalization and some doc requests.

ghost · 2024-04-24T20:32:35Z

+            // Generate `cfg(FALSE, REV1, ..., REVN)` (for all possible revisions)
+            //
+            // For compatibility reason we consider the `FALSE` cfg to be expected
+            // since it is extensively used in the testsuite.
+            check_cfg.push_str("cfg(FALSE");


Do we have any documentation for this special case? If not, could you please document this behavior in the dev-guide so that the knowledge of FALSE being "special" exists somewhere? Or rather, if it's recommended to be used stylistically for configuring something out, can that be documented too?

I'm not aware, nor was I able to find any documentation regarding FALSE being a de-facto cfg that will eval to false.

could you please document this behavior in the dev-guide

Sure, I will send a PR for that.

EDIT: forget to send it, sorry

rustc-dev-guide PR is up rust-lang/rustc-dev-guide#1966

ghost · 2024-04-24T22:27:55Z

Thanks!

@bors r+ rollup

ghost · 2024-04-24T22:27:58Z

📌 Commit b6ed813 has been approved by jieyouxu

It is now in the queue for this repository.

ghost · 2024-04-25T04:30:44Z

@bors r-
#124358 (comment)

ghost · 2024-04-25T06:08:16Z

#124358 (comment)

Hum, apparently the ui-fulldeps test suite is run with stage0, which doesn't yet know that --check-cfg has been stabilized.

Fortunately the branching of master should happen tomorrow (the 26th) and the update of stage0 sometime next week, so I think the best course of action is just to wait a couple days.

ghost · 2024-05-01T13:28:40Z

@rustbot blocked waiting for stage0 update

ghost · 2024-05-02T12:20:17Z

stage0 was updated in #124521, I rebase over it and tested it locally

@rustbot labels -S-blocked +S-waiting-on-review

ghost · 2024-05-02T15:18:20Z

Thanks!

@bors r+ rollup

ghost · 2024-05-03T07:07:32Z

Sorry this has failed in yet another rollup: #124633 (comment)

No worries. The issue was in the tests/ui/instrument-coverage/on-values.rs test which is ignored by default (requires profiler support). Fixed the test, CI passes, let's try again.

@bors r=@jieyouxu

ghost · 2024-05-03T07:07:34Z

📌 Commit 34dbfc3 has been approved by jieyouxu

It is now in the queue for this repository.

ghost · 2024-05-04T00:03:57Z

⌛ Testing commit 34dbfc3 with merge 97fd97b...

ghost · 2024-05-04T00:31:20Z

💔 Test failed - checks-actions

this directive prevents compiletest from adding any implicit and automatic --check-cfg arguments

ghost · 2024-05-04T10:28:38Z

Issue with two other tests, they were again ignored by-default, fixed them; and did a grep against the repo for similar issue and didn't find anything else. CI passes, let's try again.

@bors r=@jieyouxu

ghost · 2024-05-04T10:28:41Z

📌 Commit d4e26fb has been approved by jieyouxu

It is now in the queue for this repository.

ghost · 2024-05-04T10:31:52Z

⌛ Testing commit d4e26fb with merge 7dd170f...

ghost · 2024-05-04T12:40:34Z

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

ghost · 2024-05-04T13:56:05Z

Finished benchmarking commit (7dd170f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)	5.6%	[5.6%, 5.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

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%	[1.9%, 3.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 675.105s -> 676.427s (0.20%)
Artifact size: 315.89 MiB -> 315.91 MiB (0.01%)

rustbot assigned jieyouxu Apr 24, 2024

jieyouxu approved these changes Apr 24, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

Urgau force-pushed the compiletest-check-cfg branch from 7cefc9d to b6ed813 Compare April 24, 2024 21:03

Urgau mentioned this pull request Apr 24, 2024

Add documentation for check-cfg by-default in UI tests rust-lang/rustc-dev-guide#1966

Merged

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 Apr 24, 2024

matthiaskrgr mentioned this pull request Apr 25, 2024

Rollup of 4 pull requests #124358

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2024

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 1, 2024

Urgau force-pushed the compiletest-check-cfg branch from b6ed813 to 595ddb0 Compare May 2, 2024 12:18

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 2, 2024

jieyouxu approved these changes May 2, 2024

View reviewed changes

Urgau force-pushed the compiletest-check-cfg branch from c897ec1 to 34dbfc3 Compare May 3, 2024 05:57

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 3, 2024

This comment has been minimized.

Sign in to view

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

Urgau added 3 commits May 4, 2024 11:30

tests/ui: prepare some tests for --check-cfg by default

ed81578

compiletest: add no-auto-check-cfg directive

5173741

this directive prevents compiletest from adding any implicit and automatic --check-cfg arguments

compiletest: add enable-by-default check-cfg

d4e26fb

Urgau force-pushed the compiletest-check-cfg branch from 34dbfc3 to d4e26fb Compare May 4, 2024 09:31

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 May 4, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label May 4, 2024

bors merged commit 7dd170f into rust-lang:master May 4, 2024

rustbot added this to the 1.80.0 milestone May 4, 2024

Urgau deleted the compiletest-check-cfg branch May 4, 2024 12:41

Uh oh!

Conversation

Urgau commented Apr 24, 2024

Footnotes

Uh oh!

ghost commented Apr 24, 2024

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost Apr 24, 2024 • edited by jieyouxu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Apr 24, 2024 • edited by Urgau Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Apr 24, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

ghost commented Apr 24, 2024

Uh oh!

ghost commented Apr 24, 2024

Uh oh!

ghost commented Apr 25, 2024

Uh oh!

ghost commented Apr 25, 2024 • edited by Urgau Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 1, 2024

Uh oh!

ghost commented May 2, 2024

Uh oh!

ghost commented May 2, 2024

Uh oh!

ghost commented May 3, 2024

Uh oh!

ghost commented May 3, 2024

Uh oh!

ghost commented May 4, 2024

Uh oh!

This comment has been minimized.

ghost commented May 4, 2024

Uh oh!

ghost commented May 4, 2024

Uh oh!

ghost commented May 4, 2024

Uh oh!

ghost commented May 4, 2024

Uh oh!

ghost commented May 4, 2024

Uh oh!

ghost commented May 4, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ghost Apr 24, 2024 •

edited by jieyouxu

Loading

ghost Apr 24, 2024 •

edited by Urgau

Loading

ghost commented Apr 25, 2024 •

edited by Urgau

Loading