- Notifications
You must be signed in to change notification settings - Fork 13.9k
Run main rust-analyzer tests in rust-lang/rust CI #147372
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
base: master
Are you sure you want to change the base?
Conversation
| @bors try |
[DO NOT MERGE] Run main rust-analyzer tests try-job: aarch64-gnu try-job: aarch64-apple try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1 try-job: aarch64-msvc-1
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
9f7d76f to 43d7f3e Compare This comment was marked as outdated.
This comment was marked as outdated.
43d7f3e to bcc2aae Compare | Huh. |
[DO NOT MERGE] Run main rust-analyzer tests try-job: aarch64-gnu try-job: aarch64-apple try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1 try-job: aarch64-msvc-1
This comment has been minimized.
This comment has been minimized.
| I'd consider this a win? |
This comment was marked as resolved.
This comment was marked as resolved.
… r=Kobzol bootstrap: don't build book redirect pages during dry-run/test Currently, `./x test bootstrap` does not automatically transitively checkout submodules needed to pass all involved test steps. Apparently one place where bootstrap's self-test can choke on locally is trying to build book redirect pages without the book submodules checked out. This change is orthogonal to making bootstrap checking out required submodules for self-tests, and IMO is beneficial regardless since IMO we should not be building these redirect pages during test/dry-run _anyway_. This was blocking me trying to rebless bootstrap self-tests for rust-lang#147372. cf. [#t-infra/bootstrap > Bootstrap self-tests @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20self-tests/near/543157194). r? `@Kobzol` (or bootstrap)
Rollup merge of #147374 - jieyouxu:bootstrap-redirect-pages, r=Kobzol bootstrap: don't build book redirect pages during dry-run/test Currently, `./x test bootstrap` does not automatically transitively checkout submodules needed to pass all involved test steps. Apparently one place where bootstrap's self-test can choke on locally is trying to build book redirect pages without the book submodules checked out. This change is orthogonal to making bootstrap checking out required submodules for self-tests, and IMO is beneficial regardless since IMO we should not be building these redirect pages during test/dry-run _anyway_. This was blocking me trying to rebless bootstrap self-tests for #147372. cf. [#t-infra/bootstrap > Bootstrap self-tests @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20self-tests/near/543157194). r? `@Kobzol` (or bootstrap)
c72221d to 0af3c68 Compare | // FIXME: teach rust-analyzer to use `RUST_ANALYZER_TEST_DIR` for its test output root | ||
| // directory. In the rust-lang/rust CI, we separate checkout directory vs build directory, | ||
| // where the checkout directory is read-only whereas build directory (including test output | ||
| // directories) is writable. | ||
| let dir = testdir(builder, target); | ||
| t!(fs::create_dir_all(&dir)); | ||
| cargo.env("RUST_ANALYZER_TEST_DIR", dir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark: I added this mostly for review discussion, remove before merge if r-a wants to use another approach or don't need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, let me remove this, this was an intermediate thing. Tracking this as a follow-up in #147370.
| rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer |
0af3c68 to bbfdd24 Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look fine, but I'm not sure if this doesn't require an MCP. We should also check the CI durations.
I will open one if just for the visibility, even if in the end we find that it's not technically needed. |
bbfdd24 to bb43aca Compare bb43aca to 4762ea6 Compare | This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| The MCP rust-lang/compiler-team#923 is accepted. Pushed a rebase, this is now ready for re-review. I didn't write additional docs, because this is wired up like the usual $tool tests (i.e. |
4762ea6 to f3edba7 Compare
Part of #147370.
MCP: rust-lang/compiler-team#923
This PR prepares
rust-analyzercrates within-rust-treecargo featues where needed, and and updates bootstrap to run the mainrust-analyzertests in rust-lang/rust CI, not just theproc-macro-srvcrate tests.This supersedes the earlier attempt at #136779. I was honestly expecting more failures in this PR, but looking back at the previous attempt, that makes sense because we no longer run
i686-mingw(32-bit windows-gnu) which had a bunch of these failures. In the earlier attempt I also disabled thei686-mingw-related failures fori686-msvcsince I didn't feel like digging into 32-bit msvc at the time. Try results from this PR shows that it's most likely limited to 32-bit windows-gnu specifically.rust-analyzertest remarksCARGO_WORKSPACE_DIRexpect-test-hack in order forexpect-testto be able to find the test expectation HTML files (forsyntax_highlightingtests inide). When I added the hack, ironically, it madeexpect-testunable to find the expectation files. I think this was because previously the path was of theproc-macro-srvcrate specifically, now we point to the root r-a workspace?cfg-related differences onaarch64-apple-darwinmight've been fixed? I can't tell, but we don't seem to be observing the differences now.config::{generate_config_documentation, generate_package_json_config}no longer fails. Perhaps they were fixed to no longer try to write to source directory?Review remarks
rustc_privatecompiler crates to use thein-rust-treecargo feature. I briefly tried to use a plain--cfg=in_rust_tree, but quickly realized it was very hacky, and needed invasive bootstrap changes. The cargo feature approach seems most "natural"/well-supported to both bootstrap and cargo.proc-macro-srvtests, but the whole r-a tests.try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
try-job: aarch64-msvc-1