Migrate `validate_json.py` script to rust in `run-make/rustdoc-map-file` test by GuillaumeGomez · Pull Request #129149 · rust-lang/rust

GuillaumeGomez · 2024-08-16T10:21:35Z

This PR fixes the FIXME I added for future-me who become present-me. :')

Since there are multiple run-make tests using python scripts, I suppose more of them will migrate to Rust, hence why I added the jzon public reexport to the run-make-support crate.

cc @jieyouxu
r? @Kobzol

rustbot · 2024-08-16T10:21:43Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

The run-make-support library was changed

cc @jieyouxu

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

jieyouxu

Cool! I have a few questions about jzon and the test itself, but I'm glad it's viable to get rid of the python script :3

jieyouxu · 2024-08-16T10:36:28Z

r? jieyouxu (unless you want Kobzol to review this specifically)

GuillaumeGomez · 2024-08-16T13:12:20Z

Sum up of what I did:

I replaced jzon with serde_json (still reexported).
I stopped recursively going through folders and simply get the file from a known path (which is actually better since if we change the path, better to know it).
Removed usage of exit and only use panic instead.
Added a comment explaining what the test is checking.

GuillaumeGomez · 2024-08-16T13:13:31Z

Ah I forgot the improvement for error messages. Done as well. :)

jieyouxu

Thanks, this looks good! Just one minor style nit on the re-export. Feel free to r=me after the nit once PR CI is green.

GuillaumeGomez · 2024-08-16T13:31:17Z

Moved the reexport with the others.

…le` test

…o `run-make` tests

GuillaumeGomez · 2024-08-16T14:32:49Z

The error is super weird:

cannot determine resolution for the import

It works locally, and serde_json name is only present once at crate root. I rebased just in case...

jieyouxu · 2024-08-16T17:07:07Z

... but that seems to work now, so... 🤷

@bors r+ rollup

bors · 2024-08-16T17:07:10Z

📌 Commit 6b1637c has been approved by jieyouxu

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#128990 (Re-enable more debuginfo tests on freebsd) - rust-lang#129042 (Special-case alias ty during the delayed bug emission in `try_from_lit`) - rust-lang#129086 (Stabilize `is_none_or`) - rust-lang#129149 (Migrate `validate_json.py` script to rust in `run-make/rustdoc-map-file` test) - rust-lang#129154 (Fix wrong source location for some incorrect macro definitions) - rust-lang#129161 (Stabilize std::thread::Builder::spawn_unchecked) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129149 - GuillaumeGomez:migrate-python-script, r=jieyouxu Migrate `validate_json.py` script to rust in `run-make/rustdoc-map-file` test This PR fixes the FIXME I added for future-me who become present-me. :') Since there are multiple `run-make` tests using python scripts, I suppose more of them will migrate to Rust, hence why I added the `jzon` public reexport to the `run-make-support` crate. cc `@jieyouxu` r? `@Kobzol`

Zalathar · 2024-08-17T02:26:26Z

Heh, I was also working on a version of this, and only noticed when my own changes wouldn't rebase cleanly onto master. 😅

My approach was to add a Diff::normalize_json method that pretty-prints the "actual" text before diffing it, which lets us get rid of all the ad-hoc JSON-diffing code. I might still write up my own PR because I think it's a modest improvement over this version, and a similar technique can be used to get rid of the (trivial) Python script in run-make/libtest-json.

GuillaumeGomez · 2024-08-17T09:04:54Z

I'm not sure it's worth it to have anything more advanced than this but don't let it stop you. 😉

tgross35 · 2024-08-17T12:43:10Z

Bors is not correctly removing items from the queue

@bors r-

rustbot assigned Kobzol Aug 16, 2024

GuillaumeGomez changed the title ~~Migrate validate_json.py script to rust in run-make/rustdoc-map-file test~~Migrate validate_json.py script to rust in run-make/rustdoc-map-file testAug 16, 2024

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 16, 2024

jieyouxu reviewed Aug 16, 2024
View reviewed changes

rustbot assigned jieyouxu and unassigned Kobzol Aug 16, 2024

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 16, 2024

GuillaumeGomez force-pushed the migrate-python-script branch from ec7e0ea to b7de0ccCompare August 16, 2024 13:10

GuillaumeGomez force-pushed the migrate-python-script branch from b7de0cc to 7116ed4Compare August 16, 2024 13:13

jieyouxu approved these changes Aug 16, 2024
View reviewed changes

Comment threadsrc/tools/run-make-support/src/lib.rs Outdated

GuillaumeGomez force-pushed the migrate-python-script branch from 7116ed4 to 4cbd9ceCompare August 16, 2024 13:31

jieyouxu reviewed Aug 16, 2024
View reviewed changes

Comment threadsrc/tools/run-make-support/src/lib.rs Outdated

This comment has been minimized.
Sign in to view

GuillaumeGomez added 2 commits August 16, 2024 16:32

Migrate validate_json.py script to rust in `run-make/rustdoc-map-fi…
a2a4f2b
…le` test

Reexport serde_json crate from run-make-support to give it access t…
6b1637c
…o `run-make` tests

GuillaumeGomez force-pushed the migrate-python-script branch from 4cbd9ce to 6b1637cCompare August 16, 2024 14:32

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

matthiaskrgr mentioned this pull request Aug 16, 2024
Rollup of 6 pull requests #129162
Merged

bors merged commit 7177a0e into rust-lang:masterAug 16, 2024

rustbot added this to the 1.82.0 milestone Aug 16, 2024

GuillaumeGomez deleted the migrate-python-script branch August 16, 2024 22:51

tgross35 mentioned this pull request Aug 17, 2024
Rollup of 8 pull requests #129197
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 Aug 17, 2024

Uh oh!

Uh oh!

Conversation

GuillaumeGomez commented Aug 16, 2024

Uh oh!

rustbot commented Aug 16, 2024

Uh oh!

jieyouxu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 16, 2024

Uh oh!

GuillaumeGomez commented Aug 16, 2024

Uh oh!

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 16, 2024

Uh oh!

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Aug 16, 2024

Uh oh!

jieyouxu commented Aug 16, 2024

Uh oh!

bors commented Aug 16, 2024

Uh oh!

Zalathar commented Aug 17, 2024

Uh oh!

GuillaumeGomez commented Aug 17, 2024

Uh oh!

tgross35 commented Aug 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jieyouxu left a comment •
edited
Loading

jieyouxu commented Aug 16, 2024•
edited
Loading