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

rustdoc-json: replace jsondocck with jsondocckng #94140

Open
8 tasks
aDotInTheVoid opened this issue Feb 19, 2022 · 5 comments
Open
8 tasks

rustdoc-json: replace jsondocck with jsondocckng #94140

aDotInTheVoid opened this issue Feb 19, 2022 · 5 comments
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

We want to use https://github.com/aDotInTheVoid/jsondocckng which allows writing the tests in a seperate rust files, instread of the current jsondocck and check_missing_items.py

Reasons to do this

  • Easier for new contributors: They don't need to learn how to use JsonPath and @set... commands
  • Easier for exising contributors: Common idioms ("$.index[*][?(@.name=='...')]) can be converted to rust code to avoid duplication. Paths referenced repeatedly can be replaced with a let
  • We can avoid the problems with quoting of the shlex/serde_json frankensteined value system that leads to things like '"\"vectorcall\""'

Drawbacks

  • We dont test the raw json, but instead the Rust Repr
  • Unclear how to integrate into rustbuild in current from
  • Existing tests need to be rewritten

Todo

  • Improve features of jsondocckng
    • Allow multifile crates
    • Allow passing args to rustdoc
    • Allow multiple json outputs
    • When failure occours, diff with nightly and prity print
  • Integrate with rustbuild
  • Port existing tests
  • Document

@rustbot modify labels: +A-contributor-roadblock +A-testsuite +T-rustdoc +A-rustdoc-json +A-rustbuild

@rustbot rustbot added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 19, 2022
@aDotInTheVoid
Copy link
Member Author

aDotInTheVoid commented Feb 22, 2022

2 Possible approaches for this:

1 Crate per test

Each test is its own crate (in addition to the crate being documented). Rustbuild documents the <name>.rs file, compiles <name>_test.rs to a binary, and then calls generated binary with a path the the json. Their is a library crate that each test depends on that provides common test utilitys, and a main method that reads the json file, and then calls the test crates test function.

More concretely

foo.rs

pub fn foo() {}

foo_test.rs

use jsondocck::{TCrate, main_test}

fn main() {main_test(test);}

fn test(t: TCrate) {
   let foo = t.get_root::<Function>("foo");
   assert_eq!(foo.decl.output, None);
}

One crate for all tests

All tests live in the same (binary) crate. rustbuild the test document and builds the one test crate, and then calls the test crate binary with the name of the test to run and the path to json. The test crate has the all the test functions within it, and calls the appropried one based on the name of the test passed on the command line

This feels simpler to impement, but may lead to a slower feedback loop, as the whole test crate needs to be recompilled whenever the contents of a single test is changed.

I'm not sure which direction to go here, and would realy apprciate input

@CraftSpider
Copy link
Contributor

Personally, 1 sounds better. I like the idea of one crate per test, with a helper they all rely on. On the other hand I agree it may be a little more complex.

@aDotInTheVoid
Copy link
Member Author

After some zulip discussion with @_Mark-Simulacrum, it looks like one-crate-per-test is infeasable, because compiletest can't build crates that have crates.io dependencys (needed for serde_json).

Additionaly this looks alot like #40713

@aDotInTheVoid
Copy link
Member Author

@jyn514 jyn514 removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 11, 2022
@aDotInTheVoid
Copy link
Member Author

It might be worth just rewriting src/etc/check_missing_items.py to start with, as that should fit nicely into rustbuild, and it's out of sync whenever the format changes (and even when it doesn't due to undertesting). Eg

pub type RefFn<'a> = &'a dyn for<'b> Fn(&'a i32) -> i32;

fails it.

Theirs a very barebones implementation here.

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 20, 2023
dependencies: reduce the amount of crates pulling in atty

It would be nice to have only one `hermit-abi` in `Cargo.lock` (rust-lang#107405 (comment)).

The only crate pulling in the old `hermit-abi` version is `atty`, which is unmaintained.

This PR upgrades three dependencies, which then no longer depend on `atty`:
* `Cargo.lock`: `colored v2.0.0 -> v2.0.4`
* `Cargo.lock`: `tracing-tree v0.2.3 -> v0.2.4`
* Miri: `env_logger 0.9.3 -> 0.10.0`

The only dependency chain left that pulls in `hermit-abi 0.1.19` is:
`hermit-abi 0.1.19` -> `atty 0.2.14` -> `env_logger 0.7.1` -> `jsonpath_lib 0.2.6` -> `jsondocck 0.1.0` (src/tools/jsondocck)

Replacing jsondocck with jsondocckng is tracked in rust-lang#94140.
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 21, 2023
dependencies: reduce the amount of crates pulling in atty

It would be nice to have only one `hermit-abi` in `Cargo.lock` (rust-lang/rust#107405 (comment)).

The only crate pulling in the old `hermit-abi` version is `atty`, which is unmaintained.

This PR upgrades three dependencies, which then no longer depend on `atty`:
* `Cargo.lock`: `colored v2.0.0 -> v2.0.4`
* `Cargo.lock`: `tracing-tree v0.2.3 -> v0.2.4`
* Miri: `env_logger 0.9.3 -> 0.10.0`

The only dependency chain left that pulls in `hermit-abi 0.1.19` is:
`hermit-abi 0.1.19` -> `atty 0.2.14` -> `env_logger 0.7.1` -> `jsonpath_lib 0.2.6` -> `jsondocck 0.1.0` (src/tools/jsondocck)

Replacing jsondocck with jsondocckng is tracked in rust-lang/rust#94140.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants