Spurious RLS test failures

[RalfJung]

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]

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

[Centril]

#62209 (comment) also seems spurious.

[RalfJung]

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]

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.

[RalfJung]

@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]

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]

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