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

ui test error output doesn't play nicely with "goto file" in vscode #109725

Closed
lcnr opened this issue Mar 29, 2023 · 4 comments · Fixed by #110115
Closed

ui test error output doesn't play nicely with "goto file" in vscode #109725

lcnr opened this issue Mar 29, 2023 · 4 comments · Fixed by #110115
Assignees
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Mar 29, 2023

it seems like the source directory in the printout for ui tests uses "fake-test-src-base" rn, e.g.

---- [ui] tests/ui/traits/new-solver/prefer-param-env-on-ambiguity.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/home/lcnr/rust6/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/lcnr/rust6/tests/ui/traits/new-solver/prefer-param-env-on-ambiguity.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--remap-path-prefix=/home/lcnr/rust6/tests/ui=fake-test-src-base" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/home/lcnr/rust6/build/x86_64-unknown-linux-gnu/test/ui/traits/new-solver/prefer-param-env-on-ambiguity" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/home/lcnr/rust6/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/lcnr/rust6/build/x86_64-unknown-linux-gnu/test/ui/traits/new-solver/prefer-param-env-on-ambiguity/auxiliary" "-Ztrait-solver=next"
stdout: none
--- stderr -------------------------------
error[E0283]: type annotations needed: cannot satisfy `T: Foo<'a>`
  --> fake-test-src-base/traits/new-solver/prefer-param-env-on-ambiguity.rs:7:34
   |
LL | impl<'a, T: Bar<'a>> Foo<'a> for T {}
   |                                  ^
   |
   = note: cannot satisfy `T: Foo<'a>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0283`.
------------------------------------------

this prevents ctrl+leftclick in vscode to take me directly to the relevant point in the source of the failing test.

it would be nice to somehowo keep the actual path in the output for ui tests and only normalize it when writing into the stderr file?

see https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/ui.20tests.20.22fake-test-src-base.22 for more details

@lcnr lcnr added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Mar 29, 2023
@jyn514
Copy link
Member

jyn514 commented Mar 29, 2023

Note that we already print the real path in the "--- [ui] ... stdout ---" line at the top, but it would be nice to have this in addition so that line/column jumping works too.

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Apr 3, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 3, 2023

One possible workaround is to only pass remap-path-prefix in CI. I think it may also be possible to solve the problem on #105924 with a different mechanism entirely by stripping out the the directory with test normalization for UI tests involving closures?

https://rustc-dev-guide.rust-lang.org/tests/ui.html#normalization

@jyn514 jyn514 added the E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. label Apr 3, 2023
@TimNN
Copy link
Contributor

TimNN commented Apr 3, 2023

I think it may also be possible to solve the problem on #105924 with a different mechanism entirely by stripping out the the directory with test normalization for UI tests involving closures?

I think there are two reasons why this may be a suboptimal solution:

  1. You'd still have to deal with the = note: the full name for the casted type has been written to ... showing up depending on the path length.

  2. The difference between $DIR/fallback-closure-wrap.rs and fallback-closure-wrap.rs is that the $DIR version indicates that rustc has printed a full path, whereas for the non-$DIR version only a filename was printed. It may be useful / desirable to be able to distinguish the two cases in the output files.

@jyn514
Copy link
Member

jyn514 commented Apr 3, 2023

👍 makes sense - in that case the simplest fix is to gate remap-path-prefix on CiEnv::current() != None

@jyn514 jyn514 added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Apr 3, 2023
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@jyn514 jyn514 self-assigned this Apr 9, 2023
@bors bors closed this as completed in e327487 Apr 10, 2023
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-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants