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

Spurious RLS test failures #62225

Closed
RalfJung opened this issue Jun 29, 2019 · 8 comments
Closed

Spurious RLS test failures #62225

RalfJung opened this issue Jun 29, 2019 · 8 comments
Labels
A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jun 29, 2019

The RLS tests regularly fail on the tools builder even when the PR didn't touch anything RLS-related, and then the next PR landing switches the toolstate back to test-pass again. This is particularly bad during the week before the beta cutoff, where such a test failure leads to the PR being rejected.

I have seen this for weeks or even months, but to my surprise could not find an issue tracking this yet.
Cc @rust-lang/infra @Xanewok

Some examples (with failing platform and test):
#62209 (comment) (Linux, client_changing_workspace_lib_retains_diagnostics)
#61199 (comment) (Linux, client_find_definitions)
#61891 (comment) (Windows, client_completion_suggests_arguments_in_statements)
#61932 (comment) (Linux, client_completion_suggests_arguments_in_statements)
#61771 (comment) (Linux, client_completion_suggests_arguments_in_statements)
#61889 (comment) (Windows, client_completion_suggests_arguments_in_statements)
#60730 (comment) (Windows, client_completion_suggests_arguments_in_statements)
#59752 (comment) (Linux, client_completion_suggests_arguments_in_statements)
#61817 (comment) (Linux, client_find_definitions)
#61789 (comment) (Linux, client_completion_suggests_arguments_in_statements)
#61722 (comment) (Linux, client_completion_suggests_arguments_in_statements)
#61758 (comment) (Linux, client_find_definitions)

And that's just for the last 17 days!

@kennytm
Copy link
Member

kennytm commented Jun 29, 2019

The tracking issue is in rust-lang/rls#1265.

@Centril
Copy link
Contributor

Centril commented Jun 29, 2019

#62209 (comment) also seems spurious.

@RalfJung
Copy link
Member Author

RalfJung commented Jun 29, 2019

Indeed, and it's an interesting one: client_changing_workspace_lib_retains_diagnostics failed, which is not in my list above. (Now it is.)

So this bug has already caused two PRs to fail to land within the last 24h.

@Xanewok
Copy link
Member

Xanewok commented Jun 29, 2019

Merged RLS test fixes:

  • 19 days ago landed Update RLS #61694 which should've fixed stomping tests due to shared TARGET_DIR, which specifically fixed client_changing_workspace_lib_retains_diagnostics
  • 10 days ago landed Update rustfmt and rls #61861 which ignored client_completion_suggests_arguments_in_statements for now

#62209 (comment) (Linux, client_changing_workspace_lib_retains_diagnostics)

Failed because it's a stable backport, which doesn't yet include #61694

client_completion_suggests_arguments_in_statements

Double-checked and every failure is prior to merging #61861

...which means that the only spurious test failure in the past 10 days is #61199 (comment) (the other two instances of client_find_definitions were 15 and 17 days ago) so this isn't nearly as severe now as it was before.

It looks like client_find_definitions remains to be the only spurious test now.

@jonas-schievink jonas-schievink added A-rls A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels Jun 29, 2019
@RalfJung
Copy link
Member Author

@Xanewok thanks for the update, good to hear that client_completion_suggests_arguments_in_statements was disabled for now.

Do you have any inkling as to what might cause these failures?

@Xanewok
Copy link
Member

Xanewok commented Jun 29, 2019

I have a hunch that we might have a race in Racer (heh) + RLS in client_completion_suggests_arguments_in_statements but what's interesting is that it seems to be the only test relying on Racer that's failing intermittently

About client_find_definitions I can't say much - IIRC it's only data from save-analysis, I didn't go into that yet. Might be something inside rls-analysis (lowering save-analysis into DB-like index) but again didn't check it thoroughly yet

@RalfJung
Copy link
Member Author

RalfJung commented Feb 3, 2020

FWIW, this seems to be still happening:
#68778 (comment)

@ehuss
Copy link
Contributor

ehuss commented Aug 28, 2022

Closing as #100863 has removed RLS.

@ehuss ehuss closed this as completed Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants