-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
[DO NOT MERGE] ./x test rust-analyzer
#136779
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry Albert, this PR is purely for reference purposes and is highly experimental. |
[features] | ||
default = [] | ||
in-rust-tree = [] |
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.
Maybe could be --cfg
instead but idk
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.
I was hoping we could make this a cfg at some point. Either way out of scope for this PR
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.
It probably can, I just yolo'd it as cargo features to get it to build first of all.
// RA's test suite tries to write to the source directory, that can't work in Rust CI. | ||
cargo.env("SKIP_SLOW_TESTS", "1"); |
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.
Note that this will mean not all r-a tests would be run via ./x test rust-analyzer
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.
Is there some blessed directory where writes are allowed? (I believe we are trying to write to temp dirs)
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.
Yes, bootstrap has a builder.out
(which is basically some kind of build/
directory). Under this, the structure could be something like (I only know exactly the compiletest-managed output dir shape, I don't quite remember the crate unit test output dir shape).
build/
test/
rust-analyzer/...
TL;DR: sure, if the writes are done specifically against such build output dir (for exmaple, run-make tests are allowed to do writes to such per-run-make-test output 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.
In r-l/r CI, the build output dir doesn't overlap with source checkout dir, where the source dir is read-only.
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.
Gotcha, sounds like something that ought to be easily supported but can also be done as a follow up (given it needs touch up on the r-a side)
/// When using `download-rustc`, we need to use a new build of `std` for running unit tests of | ||
/// std itself, but we need to use the downloaded copy of std for linking to rustdoc. Allow this | ||
/// to be overridden by `builder.ensure` from other steps. |
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.
Unrelated comment reflow diffs, was annoying when reading these locally, I'll drop these formatting diffs if we want to merge the actual changes.
skip_all, | ||
fields(target = ?self.target, compiler = ?self.compiler), | ||
), | ||
)] | ||
fn run(self, builder: &Builder<'_>) -> u32 { | ||
let compiler = self.compiler; |
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.
On Rustc
there's a comment on self.compiler
that says
/// The **previous** compiler used to compile this compiler.
pub compiler: Compiler,
I'm not 100% sure if that's actually true under all scenarios.
☔ The latest upstream changes (presumably #136751) made this pull request unmergeable. Please resolve the merge conflicts. |
(Uh sorry Onur, I have no idea how I managed to click that button, did not mean to do that) |
// FIXME: may need a fix from https://github.com/rust-lang/rust-analyzer/pull/19124. | ||
"--skip=config::tests::cargo_target_dir_subdir", |
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.
Other tests pass locally, except for the test fixed in rust-lang/rust-analyzer#19124 (which can be re-eval'd after the next subtree sync)
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
[DO NOT MERGE] `./x test rust-analyzer` I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?** Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up. Notes: - I abused a bunch of cargo features `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle. - I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures. ```rs #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[cfg(all(feature = "in-rust-tree", test))] extern crate rustc_driver as _; ``` - Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet. [^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet. try-job: aarch64-gnu try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-mingw-1 try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1
This comment was marked as off-topic.
This comment was marked as off-topic.
@bors try |
💔 Test failed - checks-actions |
@bors try |
[DO NOT MERGE] `./x test rust-analyzer` I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?** Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up. Notes: - I abused a bunch of cargo features `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle. - I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures. ```rs #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[cfg(all(feature = "in-rust-tree", test))] extern crate rustc_driver as _; ``` - Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet. [^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet. try-job: aarch64-gnu try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-mingw try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1
This comment has been minimized.
This comment has been minimized.
@bors try |
[DO NOT MERGE] `./x test rust-analyzer` I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?** Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up. Notes: - I abused a bunch of cargo features `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle. - I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures. ```rs #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[cfg(all(feature = "in-rust-tree", test))] extern crate rustc_driver as _; ``` - Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet. [^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet. try-job: aarch64-gnu try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-mingw-1 try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors try |
[DO NOT MERGE] `./x test rust-analyzer` I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?** Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up. Notes: - I abused a bunch of cargo features `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle. - I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures. ```rs #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[cfg(all(feature = "in-rust-tree", test))] extern crate rustc_driver as _; ``` - Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet. [^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet. try-job: aarch64-gnu try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-mingw-1 try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
I needed to: - Consistently use `feature(rustc_private)` on all participating r-a crates under `test` scenario. - Force an `extern crate rustc_driver;` on all the r-a crates for reasons I don't yet understand. At least this builds, *at all*.
@bors try |
[DO NOT MERGE] `./x test rust-analyzer` I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?** Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up. Notes: - Unrelated tracing bits are extracted to rust-lang#137080. - I abused a bunch of cargo features `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle. - I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures. ```rs #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[cfg(all(feature = "in-rust-tree", test))] extern crate rustc_driver as _; ``` - Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet. [^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet. try-job: aarch64-gnu try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-mingw-1 try-job: x86_64-mingw-1 try-job: i686-msvc-1 try-job: x86_64-msvc-1
☀️ Try build successful - checks-actions |
#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] | ||
#[cfg(all(feature = "in-rust-tree", test))] | ||
extern crate rustc_driver as _; |
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.
Will we need to add this prelude for any new crate we add, or only for those that have (unit) tests?
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.
Only for new crates that have unit tests AFAIK. What I did basically was to tag all the crates that had unit tests (which complained about not finding rustc_lexer
and friends).
// RA's test suite tries to write to the source directory, that can't work in Rust CI. | ||
cargo.env("SKIP_SLOW_TESTS", "1"); |
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.
Is there some blessed directory where writes are allowed? (I believe we are trying to write to temp dirs)
// FIXME: fails on `aarch64-apple` due to `cfg` differences | ||
"item_tree::tests::generics_with_attributes", | ||
"hover::tests::generic_params_disabled_by_cfg", |
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.
This seems very odd, I don't see how that could ever happen (you don't perhaps recall the exact failure here?)
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.
I'll have to dig through the logs here, but yes, I believe it was sometimes like S
vs S<{unknown}>
or sth
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.
I can't seem to find the exact log, I'll comment these out then re-run a try-job
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.
Ah you've written it down in the x86_64-apple-darwin failure (which lists these two too),
// FIXME: missing
#[cfg(never)]
on struct generic param
That seems ... worrying. We don't do any actual resolution here or anything, we merely record these and then print them again ...
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 I did find it:
failures:
---- item_tree::tests::generics_with_attributes stdout ----
�[1m�[91merror�[97m: expect test failed�[0m
�[1m�[34m-->�[0m src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs:413:9
You can update all `expect!` tests by running:
env UPDATE_EXPECT=1 cargo test
To update a single test, place the cursor on `expect` token and use `run` feature of rust-analyzer.
�[1mExpect�[0m:
----
// AstId: 1
pub(self) struct S<#[cfg(never)] T>;
// AstId: 2
pub(self) struct S<A, B, #[cfg(never)] C>;
// AstId: 3
pub(self) struct S<A, #[cfg(never)] B, C>;
----
�[1mActual�[0m:
----
// AstId: 1
pub(self) struct S<T>;
// AstId: 2
pub(self) struct S<A, B, C>;
// AstId: 3
pub(self) struct S<A, B, C>;
----
�[1mDiff�[0m:
----
// AstId: 1
pub(self) struct S<�[4m�[31m#[cfg(never)] �[0mT>;
// AstId: 2
pub(self) struct S<A, B,�[4m�[31m #[cfg(never)]�[0m C>;
// AstId: 3
pub(self) struct S<A,�[4m�[31m #[cfg(never)]�[0m B, C>;
----
failures:
item_tree::tests::generics_with_attributes
- https://github.com/rust-lang-ci/rust/actions/runs/13245871780/job/36972138747
- https://productionresultssa16.blob.core.windows.net/actions-results/a0064332-2b60-493e-823e-4af3c8b0e26f/workflow-job-run-ae0ef69a-61cf-5efc-bd6c-9c5358e76cef/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-02-16T10%3A45%3A02Z&sig=Qta6%2FiPBYJFQbDtGvr%2BW8x0ytW%2BLXCihrgtCvkK3FzA%3D&ske=2025-02-16T20%3A29%3A55Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-02-16T08%3A29%3A55Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-01-05&sp=r&spr=https&sr=b&st=2025-02-16T10%3A34%3A57Z&sv=2025-01-05
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.
So yeah, this one was actually #[cfg(never)]
seems to have disappeared on aarch64-apple-darwin
(and also x86_64-apple-darwin
IIRC)
// FIXME: I think this exercises cargo somehow? | ||
"tests::smoke_test_real_sysroot_cargo", |
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.
This invokes rustc --print sysroot
and then runs cargo metadata
on the sysroot (I did notice a bug here though I don't think thats relevant rust-lang/rust-analyzer#19159)
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.
It's possible that rustc
doesn't go through the rustc wrapper, and maybe invoking a wrong rustc or sth, which maybe reports a wrong/different sysroot?
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.
Yes, from a quick glance when we invoke rustc we seem to consider the following for rustc (in that order):
- Check for the
RUSTC
env var - Check if
rustc
is inPATH
- Check if
rustc
exists as a proxy in<cargo-home>/bin/rustc
- Otherwise we'll just invoke
rustc
as is
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.
I think what I'll end up doing is to open a tracking issue for ./x test rust-analyzer
(in rust-lang/rust), and try to land a minimal version of it (with most of these skips intact) to make it possible to run try-jobs against r-l/r's CI, and track some of these tests being disabled separately.
That way, if you really wanted to, you can also try to make changes to r-a then run them against aarch64-apple-darwin
under r-l/r's CI :ferrisClueless:
For instance, some of these need-to-control-environment tests seem less urgent, so could probably be fixed separately.
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.
Yes, I don't think we want to fix all of these tests. Most of these are const eval related and that infra is semi broken anyways (I imagine we make 64 bit layout assumptions in some places accidentally)
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.
Yeah makes sense. I'll wait for #137080 to land first, then I'll pull the actual r-a test bits out into a separate PR.
// FIXME: config generation | ||
"config::tests::generate_config_documentation", | ||
"config::tests::generate_package_json_config", |
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.
I assume these fail because they try to write to the source dir again
"inlay_hints::discriminant::tests::datacarrying_mixed", | ||
"inlay_hints::discriminant::tests::fieldless", | ||
// FIXME: list order non-deterministic or different on `i686-mingw`? | ||
"runnables::tests::tests_are_unique", |
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.
rust-lang/rust-analyzer#19161 should fix this
// FIXME: annotation order seems to be different on `i686-mingw` | ||
"annotations::tests::runnable_annotation", | ||
"annotations::tests::struct_references_annotations", | ||
"annotations::tests::struct_and_trait_impls_annotations", | ||
"annotations::tests::method_annotations", |
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.
rust-lang/rust-analyzer#19161 should fix this
EDIT 2: that turned out to be very hard to cleanly organize so nvm. |
I somehow made
./x test rust-analyzer
work on my machine1, but at what cost?Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up.
Notes:
in-rust-tree
. It probably doesn't need to be, and simply--cfg
might work. I was trying to get the main rust-analyzer tests to build at all. Anything building is already a miracle.expected `rustc_lexer` to be available in rlib format
build failures.try-job: aarch64-gnu
try-job: x86_64-apple-1
try-job: aarch64-apple
try-job: i686-mingw-1
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
Footnotes
x86_64-unknown-linux-gnu
, haven't bothered trying this on msvc yet. ↩