Rustdoc json tests: New @ismany test command #99474

aDotInTheVoid · 2022-07-19T16:00:44Z

Alot of the time, we wanted to assert that a module had an exact set of items. Most of the time this was done by asserting that the @count of the module was n, and then doing n @has checks on the module.

This was tedious, so often shortcuts were done.

This PR adds a new command to jsondocck to allow consistently expressing this behavior, and then uses it to clean up the tests.

@rustbot modify labels: +A-rustdoc-json +A-testsuite

rust-highfive · 2022-07-19T16:00:47Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

Enselic

I skimmed it over and it looks like a nice simplification/improvement. I left some comments. Mostly nit-picks I'm afraid (as in: I probably should not have bothered to comment at all)

Enselic · 2022-07-19T18:51:06Z

src/test/rustdoc-json/reexport/glob_extern.rs

@@ -4,15 +4,20 @@
 #![feature(no_core)]

 // @is glob_extern.json "$.index[*][?(@.name=='mod1')].kind" \"module\"
-// @is glob_extern.json "$.index[*][?(@.name=='mod1')].inner.is_stripped" "true"
+// @is - "$.index[*][?(@.name=='mod1')].inner.is_stripped" "true"


Sorry to nit-pick, but I wonder: Is it worth to use - instead of the whole filename? That messes up the alignment and makes the code harder to read, IMHO.

Edit: Well, if all other lines use - then the alignment becomes good for them at least...

Yeah, most of the tests use - for all but the first item. (Side note, I should add a feature to set the - to the obvious value initialy, because I don't think this is ever used, but just a hold over from the HTML side)

src/tools/jsondocck/src/main.rs

Enselic · 2022-07-19T18:55:50Z

src/tools/jsondocck/src/main.rs

@@ -50,13 +50,15 @@ pub enum CommandKind {
    Has,
    Count,
    Is,
+    HasExact,


Bikeshedding: Maybe call it "HasOnly"? That makes more semantic sense to me, but not sure it makes more sense to others.

src/tools/jsondocck/src/main.rs

src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs

src/test/rustdoc-json/reexport/glob_private.rs

src/test/rustdoc-json/reexport/simple_private.rs

Mark-Simulacrum · 2022-07-23T22:32:06Z

src/tools/jsondocck/src/main.rs

+
+            // Serde json doesn't implement Ord or Hash for Value, so we must
+            // use a Vec here. While in theory that makes setwize equality
+            // O(n^2), in practice n will never be large enought to matter.


I feel like a 'hasexact' would make me expect that it would check the order too, not just presence -- and that would avoid any question of n^2 comparison time. I agree that n^2 comparison time isn't itself an issue, though, so maybe this is fine as-is.

It needs to not check order, as that's not a property that's stable, so the intension is to a setwise comparison.

HasExact should definatly be renamed, as it's been confusing twice now.

Mark-Simulacrum · 2022-07-23T22:32:23Z

r? rust-lang/rustdoc

notriddle · 2022-07-26T20:58:11Z

r? @CraftSpider

CraftSpider · 2022-08-08T21:03:03Z

Looked it over, and the code looks good to me, aside from the mention of HasExact being not a great name. Maybe something like ContainsExact or ItemsExact? Otherwise, r=me.

camelid · 2022-08-09T18:22:39Z

I suggested the name @hasmany on Zulip, which I think captures the meaning well.

CraftSpider · 2022-08-11T19:51:44Z

I see today that it looks like ismany was the mentioned final choice - once that is in, I'll gladly approve this change.

aDotInTheVoid · 2022-08-12T17:13:26Z

Renamed to @ismany

@rustbot ready

CraftSpider · 2022-08-16T17:07:58Z

@bors r+

bors · 2022-08-16T17:08:00Z

📌 Commit 760b972 has been approved by CraftSpider

It is now in the queue for this repository.

…-test-cleanup, r=CraftSpider Rustdoc json tests: New @hasexact test command Alot of the time, we wanted to assert that a module had an exact set of items. Most of the time this was done by asserting that the ``@count`` of the module was `n`, and then doing `n` ``@has`` checks on the module. This was tedious, so often shortcuts were done. This PR adds a new command to jsondocck to allow consistently expressing this behavior, and then uses it to clean up the tests. `@rustbot` modify labels: +A-rustdoc-json +A-testsuite

…iaskrgr Rollup of 15 pull requests Successful merges: - rust-lang#99474 (Rustdoc json tests: New `@hasexact` test command) - rust-lang#99972 (interpret: only consider 1-ZST when searching for receiver) - rust-lang#100018 (Clean up `LitKind`) - rust-lang#100379 (triagebot: add translation-related mention groups) - rust-lang#100389 (Do not report cycle error when inferring return type for suggestion) - rust-lang#100489 (`is_knowable` use `Result` instead of `Option`) - rust-lang#100532 (unwind: don't build dependency when building for Miri) - rust-lang#100608 (needless separation of impl blocks) - rust-lang#100621 (Pass +atomics-32 feature for {arm,thumb}v4t-none-eabi) - rust-lang#100646 (Migrate emoji identifier diagnostics to `SessionDiagnostic` in rustc_interface) - rust-lang#100652 (Remove deferred sized checks (make them eager)) - rust-lang#100655 (Update books) - rust-lang#100656 (Update cargo) - rust-lang#100660 (Fixed a few documentation errors) - rust-lang#100661 (Fixed a few documentation errors) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Rustdoc-Json: Clean up tests

dacffd2

rust-highfive assigned Mark-Simulacrum Jul 19, 2022

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 19, 2022

rustbot added A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc labels Jul 19, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2022

This comment has been minimized.

Sign in to view

Enselic reviewed Jul 19, 2022

View reviewed changes

aDotInTheVoid commented Jul 19, 2022

View reviewed changes

aDotInTheVoid force-pushed the rustdoc-json-noinline-test-cleanup branch from feff909 to 6d2104b Compare July 19, 2022 21:32

This comment has been minimized.

Sign in to view

aDotInTheVoid force-pushed the rustdoc-json-noinline-test-cleanup branch from 6d2104b to 09d080e Compare July 20, 2022 08:57

Mark-Simulacrum reviewed Jul 23, 2022

View reviewed changes

rust-highfive assigned notriddle and unassigned Mark-Simulacrum Jul 23, 2022

rust-highfive assigned CraftSpider and unassigned notriddle Jul 26, 2022

aDotInTheVoid force-pushed the rustdoc-json-noinline-test-cleanup branch from 09d080e to 0e563c8 Compare August 12, 2022 17:11

Jsondocck: New @ismany command

64f790f

aDotInTheVoid force-pushed the rustdoc-json-noinline-test-cleanup branch from 0e563c8 to 5b5c1fa Compare August 12, 2022 17:12

This comment has been minimized.

Sign in to view

rustdoc-json: Use @ismany in tests

760b972

aDotInTheVoid force-pushed the rustdoc-json-noinline-test-cleanup branch from 5b5c1fa to 760b972 Compare August 12, 2022 22:56

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 Aug 16, 2022

matthiaskrgr mentioned this pull request Aug 17, 2022

Rollup of 12 pull requests #100664

Closed

matthiaskrgr mentioned this pull request Aug 17, 2022

Rollup of 15 pull requests #100677

Merged

aDotInTheVoid changed the title ~~Rustdoc json tests: New @hasexact test command~~ Rustdoc json tests: New @IsMany test command Aug 17, 2022

aDotInTheVoid changed the title ~~Rustdoc json tests: New @IsMany test command~~ Rustdoc json tests: New @ismany test command Aug 17, 2022

bors merged commit 0491fda into rust-lang:master Aug 17, 2022

rustbot added this to the 1.65.0 milestone Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rustdoc json tests: New @ismany test command #99474

Rustdoc json tests: New @ismany test command #99474

aDotInTheVoid commented Jul 19, 2022

rust-highfive commented Jul 19, 2022

This comment has been minimized.

Enselic left a comment •

edited

Loading

Enselic Jul 19, 2022 •

edited

Loading

aDotInTheVoid Jul 19, 2022

Enselic Jul 19, 2022

This comment has been minimized.

Mark-Simulacrum Jul 23, 2022

aDotInTheVoid Jul 25, 2022

Mark-Simulacrum commented Jul 23, 2022

notriddle commented Jul 26, 2022

CraftSpider commented Aug 8, 2022

camelid commented Aug 9, 2022

CraftSpider commented Aug 11, 2022

aDotInTheVoid commented Aug 12, 2022

This comment has been minimized.

CraftSpider commented Aug 16, 2022

bors commented Aug 16, 2022

Rustdoc json tests: New @ismany test command #99474

Rustdoc json tests: New @ismany test command #99474

Conversation

aDotInTheVoid commented Jul 19, 2022

rust-highfive commented Jul 19, 2022

This comment has been minimized.

Enselic left a comment • edited Loading

Choose a reason for hiding this comment

Enselic Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

aDotInTheVoid Jul 19, 2022

Choose a reason for hiding this comment

Enselic Jul 19, 2022

Choose a reason for hiding this comment

This comment has been minimized.

Mark-Simulacrum Jul 23, 2022

Choose a reason for hiding this comment

aDotInTheVoid Jul 25, 2022

Choose a reason for hiding this comment

Mark-Simulacrum commented Jul 23, 2022

notriddle commented Jul 26, 2022

CraftSpider commented Aug 8, 2022

camelid commented Aug 9, 2022

CraftSpider commented Aug 11, 2022

aDotInTheVoid commented Aug 12, 2022

This comment has been minimized.

CraftSpider commented Aug 16, 2022

bors commented Aug 16, 2022

Enselic left a comment •

edited

Loading

Enselic Jul 19, 2022 •

edited

Loading