Replace the metadata collector with tests by Alexendoo · Pull Request #13221 · rust-lang/rust-clippy

Alexendoo · 2024-08-05T14:28:02Z

The metadata collector handles 3 files: CHANGELOG.md, lint_configuration.md and util/gh-pages/lints.json

CHANGELOG.md and lint_configuration.md are now checked by tests/config-metadata.rs, when they are outdated cargo test will fail with a message to run cargo bless --test config-metadata in order to update them

A plain cargo bless will run all the tests, blessing both this and the UI tests

util/gh-pages/lints.json is now generated when running cargo uitest with COLLECT_METADATA=1 (still aliased to cargo collect-metadata)

It uses a ui_test post test action to retrieve the applicability from the actual diagnostics emitted during UI tests

Example change from the current to new JSON:

   {
     "id": "chars_next_cmp",
-    "id_span": {
-      "path": "src/methods/mod.rs",
-      "line": 891
-    },
+    "id_location": "clippy_lints/src/methods/mod.rs#891",
     "group": "style",
     "level": "warn",
-    "docs": "\n### What it does ... ```",
+    "docs": "### What it does ... ```\n",
     "version": "pre 1.29.0",
-    "applicability": {
-      "is_multi_part_suggestion": false,
-      "applicability": "Unresolved"
-    }
+    "applicability": "MachineApplicable"
   },

Hide whitespace makes the compile-test.rs changes much easier to see

r? @flip1995

changelog: none

tests/config-metadata.rs

flip1995

Still need to finalize my review, but overall this change makes sense to me. As the "metadata collection monster" is @xFrednet's baby, I also want to give them a chance to take a look.

xFrednet · 2024-08-06T13:54:17Z

I already saw this PR and was a bit sad to see the monster go. But it has served us well, and maybe it's time for retirement.

I've skimmed very quickly skimmed over the changes and like the idea. I'll give it a review this week :D

bors · 2024-08-06T15:15:04Z

☔ The latest upstream changes (presumably #13225) made this pull request unmergeable. Please resolve the merge conflicts.

tests/compile-test.rs

bors · 2024-08-08T17:14:00Z

☔ The latest upstream changes (presumably #13236) made this pull request unmergeable. Please resolve the merge conflicts.

xFrednet

Very nice implementation. I really like how you collected all the information! It's a bit sad to see my metadata collection monster go, but this implementation is way better IMO!

Thank you for putting in the time!

@flip1995, I've reviewed all changes. If you don't have the time, you can r=me :D

flip1995 · 2024-08-12T13:22:41Z

@bors r=xFrednet

Thanks for reviewing this! I didn't get to review it this weekend (at least not for an in-depth review). I like the new way of testing it though. Great overall improvements!

bors · 2024-08-12T13:22:43Z

📌 Commit 54a1d76 has been approved by xFrednet

It is now in the queue for this repository.

Replace the metadata collector with tests The metadata collector handles 3 files: [`CHANGELOG.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/CHANGELOG.md#L6050), [`lint_configuration.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/book/src/lint_configuration.md) and `util/gh-pages/lints.json` - `CHANGELOG.md` and `lint_configuration.md` are now checked by `tests/config-metadata.rs`, when they are outdated `cargo test` will fail with a message to run `cargo bless --test config-metadata` in order to update them A plain `cargo bless` will run all the tests, blessing both this and the UI tests - `util/gh-pages/lints.json` is now generated when running `cargo uitest` with `COLLECT_METADATA=1` (still aliased to `cargo collect-metadata`) It uses a `ui_test` [post test action](https://docs.rs/ui_test/latest/ui_test/custom_flags/trait.Flag.html#method.post_test_action) to retrieve the applicability from the actual diagnostics emitted during UI tests Example change from the current to new JSON: ```diff { "id": "chars_next_cmp", - "id_span": { - "path": "src/methods/mod.rs", - "line": 891 - }, + "id_location": "clippy_lints/src/methods/mod.rs#891", "group": "style", "level": "warn", - "docs": "\n### What it does ... ```", + "docs": "### What it does ... ```\n", "version": "pre 1.29.0", - "applicability": { - "is_multi_part_suggestion": false, - "applicability": "Unresolved" - } + "applicability": "MachineApplicable" }, ``` `Hide whitespace` makes the `compile-test.rs` changes much easier to see r? `@flip1995` changelog: none

bors · 2024-08-12T13:23:51Z

⌛ Testing commit 54a1d76 with merge 8838220...

bors · 2024-08-12T13:29:23Z

💔 Test failed - checks-action_test

xFrednet · 2024-08-12T13:36:22Z

It looks like the metadata_collector test fails for i686-unknown-linux-gnu. We can restrict that test to 64bits and linux to see if that fixes it.

Alexendoo · 2024-08-12T13:54:19Z

👀

Going to print a diff first, ideally it wouldn't be platform dependent

Alexendoo · 2024-08-12T13:55:09Z

@bors try

Replace the metadata collector with tests The metadata collector handles 3 files: [`CHANGELOG.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/CHANGELOG.md#L6050), [`lint_configuration.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/book/src/lint_configuration.md) and `util/gh-pages/lints.json` - `CHANGELOG.md` and `lint_configuration.md` are now checked by `tests/config-metadata.rs`, when they are outdated `cargo test` will fail with a message to run `cargo bless --test config-metadata` in order to update them A plain `cargo bless` will run all the tests, blessing both this and the UI tests - `util/gh-pages/lints.json` is now generated when running `cargo uitest` with `COLLECT_METADATA=1` (still aliased to `cargo collect-metadata`) It uses a `ui_test` [post test action](https://docs.rs/ui_test/latest/ui_test/custom_flags/trait.Flag.html#method.post_test_action) to retrieve the applicability from the actual diagnostics emitted during UI tests Example change from the current to new JSON: ```diff { "id": "chars_next_cmp", - "id_span": { - "path": "src/methods/mod.rs", - "line": 891 - }, + "id_location": "clippy_lints/src/methods/mod.rs#891", "group": "style", "level": "warn", - "docs": "\n### What it does ... ```", + "docs": "### What it does ... ```\n", "version": "pre 1.29.0", - "applicability": { - "is_multi_part_suggestion": false, - "applicability": "Unresolved" - } + "applicability": "MachineApplicable" }, ``` `Hide whitespace` makes the `compile-test.rs` changes much easier to see r? `@flip1995` changelog: none

bors · 2024-08-12T13:56:17Z

⌛ Trying commit a9057aa with merge 0853748...

bors · 2024-08-12T14:03:06Z

💔 Test failed - checks-action_test

Alexendoo · 2024-08-12T14:06:48Z

-**Default Value:** `["j", "z", "i", "y", "n", "x", "w"]`
+**Default Value:** `["w", "n", "x", "i", "z", "y", "j"]`

Can't say that's what I expected, I'll take a better look at this later

xFrednet · 2024-08-12T14:57:48Z

What o.O, that's weird.

The good news are that #13084 would fix it, but that PR is still WIP

Alexendoo · 2024-08-12T16:26:49Z

@bors try

bors · 2024-08-12T16:28:14Z

⌛ Trying commit d6f0a28 with merge 236a77c...

Replace the metadata collector with tests The metadata collector handles 3 files: [`CHANGELOG.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/CHANGELOG.md#L6050), [`lint_configuration.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/book/src/lint_configuration.md) and `util/gh-pages/lints.json` - `CHANGELOG.md` and `lint_configuration.md` are now checked by `tests/config-metadata.rs`, when they are outdated `cargo test` will fail with a message to run `cargo bless --test config-metadata` in order to update them A plain `cargo bless` will run all the tests, blessing both this and the UI tests - `util/gh-pages/lints.json` is now generated when running `cargo uitest` with `COLLECT_METADATA=1` (still aliased to `cargo collect-metadata`) It uses a `ui_test` [post test action](https://docs.rs/ui_test/latest/ui_test/custom_flags/trait.Flag.html#method.post_test_action) to retrieve the applicability from the actual diagnostics emitted during UI tests Example change from the current to new JSON: ```diff { "id": "chars_next_cmp", - "id_span": { - "path": "src/methods/mod.rs", - "line": 891 - }, + "id_location": "clippy_lints/src/methods/mod.rs#891", "group": "style", "level": "warn", - "docs": "\n### What it does ... ```", + "docs": "### What it does ... ```\n", "version": "pre 1.29.0", - "applicability": { - "is_multi_part_suggestion": false, - "applicability": "Unresolved" - } + "applicability": "MachineApplicable" }, ``` `Hide whitespace` makes the `compile-test.rs` changes much easier to see r? `@flip1995` changelog: none

bors · 2024-08-12T16:33:57Z

💔 Test failed - checks-action_test

Alexendoo · 2024-08-12T16:41:43Z

oops

@bors try

Replace the metadata collector with tests The metadata collector handles 3 files: [`CHANGELOG.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/CHANGELOG.md#L6050), [`lint_configuration.md`](https://github.com/rust-lang/rust-clippy/blob/c082bc2cb85313901ed3565fcd285592ed93df0f/book/src/lint_configuration.md) and `util/gh-pages/lints.json` - `CHANGELOG.md` and `lint_configuration.md` are now checked by `tests/config-metadata.rs`, when they are outdated `cargo test` will fail with a message to run `cargo bless --test config-metadata` in order to update them A plain `cargo bless` will run all the tests, blessing both this and the UI tests - `util/gh-pages/lints.json` is now generated when running `cargo uitest` with `COLLECT_METADATA=1` (still aliased to `cargo collect-metadata`) It uses a `ui_test` [post test action](https://docs.rs/ui_test/latest/ui_test/custom_flags/trait.Flag.html#method.post_test_action) to retrieve the applicability from the actual diagnostics emitted during UI tests Example change from the current to new JSON: ```diff { "id": "chars_next_cmp", - "id_span": { - "path": "src/methods/mod.rs", - "line": 891 - }, + "id_location": "clippy_lints/src/methods/mod.rs#891", "group": "style", "level": "warn", - "docs": "\n### What it does ... ```", + "docs": "### What it does ... ```\n", "version": "pre 1.29.0", - "applicability": { - "is_multi_part_suggestion": false, - "applicability": "Unresolved" - } + "applicability": "MachineApplicable" }, ``` `Hide whitespace` makes the `compile-test.rs` changes much easier to see r? `@flip1995` changelog: none

bors · 2024-08-12T16:42:50Z

⌛ Trying commit c563b9d with merge 13cc5bd...

bors · 2024-08-12T16:49:16Z

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: 13cc5bd (13cc5bd862dc9c484cd772726a5d50a1bbe7cc91)

Alexendoo · 2024-08-12T16:53:15Z

We used FxHashSet in Conf a few times, I'm guessing the order changed because usize hashes differently on 32-bit. I'll leave an actual fix to #13084 but as a temporary measure I replaced them with Vecs

xFrednet

LGTM! Could you copy the changes from #13261 to correct the indention and include compile_fail for code blocks?

xFrednet · 2024-08-13T09:35:51Z

Everything looks good to me, I only now noticed the branch name ^^

Death of a monster

Roses are red,
Violets are blue,
The collector,
is now anew.

Farewell my friend,
ḿy biggest lint,
who started this journey,
we're all in.

RIP
*2021-05-05
†2024-08-13

bors · 2024-08-13T09:35:53Z

📌 Commit a22ce2d has been approved by xFrednet

It is now in the queue for this repository.

bors · 2024-08-13T09:37:01Z

⌛ Testing commit a22ce2d with merge 61a252f...

bors · 2024-08-13T09:44:45Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 61a252f to master...

Remove unused `collect_metadata` function Leftover from #13221 changelog: none

rustbot assigned flip1995 Aug 5, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 5, 2024

Alexendoo force-pushed the farewell-metadata-collector branch from ad4b186 to 5447e8e Compare August 5, 2024 14:48

flip1995 reviewed Aug 5, 2024

View reviewed changes

tests/config-metadata.rs Show resolved Hide resolved

flip1995 mentioned this pull request Aug 6, 2024

Move internal lints to their own crate #13223

Merged

flip1995 reviewed Aug 6, 2024

View reviewed changes

xFrednet self-assigned this Aug 6, 2024

Alexendoo force-pushed the farewell-metadata-collector branch from 5447e8e to a3fbcb8 Compare August 6, 2024 21:44

Jarcho reviewed Aug 7, 2024

View reviewed changes

tests/compile-test.rs Outdated Show resolved Hide resolved

tests/compile-test.rs Outdated Show resolved Hide resolved

Alexendoo force-pushed the farewell-metadata-collector branch from a3fbcb8 to 8e66109 Compare August 7, 2024 19:07

Alexendoo force-pushed the farewell-metadata-collector branch from 8e66109 to 54a1d76 Compare August 8, 2024 17:51

xFrednet approved these changes Aug 9, 2024

View reviewed changes

Alexendoo force-pushed the farewell-metadata-collector branch from 54a1d76 to a9057aa Compare August 12, 2024 13:54

Alexendoo force-pushed the farewell-metadata-collector branch from a9057aa to d6f0a28 Compare August 12, 2024 16:26

Alexendoo force-pushed the farewell-metadata-collector branch from d6f0a28 to c563b9d Compare August 12, 2024 16:38

xFrednet mentioned this pull request Aug 12, 2024

fix code blocks in doc comments inconsistently using 3 or 4 spaces of indentation #13261

Merged

xFrednet reviewed Aug 12, 2024

View reviewed changes

Alexendoo added 2 commits August 12, 2024 20:24

Replace the metadata collector with tests

182cd5f

Remove HashSets from Conf

a22ce2d

Alexendoo force-pushed the farewell-metadata-collector branch from c563b9d to a22ce2d Compare August 12, 2024 20:25

xFrednet approved these changes Aug 13, 2024

View reviewed changes

bors merged commit 61a252f into rust-lang:master Aug 13, 2024

Alexendoo deleted the farewell-metadata-collector branch August 13, 2024 12:00

Alexendoo mentioned this pull request Sep 11, 2024

Remove unused collect_metadata function #13384

Merged

bors added a commit that referenced this pull request Sep 21, 2024

Auto merge of #13384 - Alexendoo:remove-old-collect-metadata, r=llogiq

0e1ded0

Remove unused `collect_metadata` function Leftover from #13221 changelog: none

Conversation

Alexendoo commented Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

flip1995 left a comment

Choose a reason for hiding this comment

Uh oh!

xFrednet commented Aug 6, 2024

Uh oh!

bors commented Aug 6, 2024

Uh oh!

Uh oh!

Uh oh!

bors commented Aug 8, 2024

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

flip1995 commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

xFrednet commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

xFrednet commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

Alexendoo commented Aug 12, 2024

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

xFrednet commented Aug 13, 2024

Death of a monster

Uh oh!

bors commented Aug 13, 2024

Uh oh!

bors commented Aug 13, 2024

Uh oh!

bors commented Aug 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Alexendoo commented Aug 5, 2024 •

edited

Loading