tests: Make tests reliable.

[emilio]

Properly wait until we're done indexing instead of waiting in a loop.

[emilio]

r? @Xanewok or @nrc

[Xanewok]

Unfortunately, as I understand it, sometimes RLS can't correctly answer the first time we issue the request - probably because we run the Racer completion on another thread iirc. This was explicitly introduced in #1099 and tests that it generally works, instead of fast-failing.

This isn't desired behaviour but I wouldn't reenable the fast-failing approach, not unless we tackle the underlying Racer completion empty response issue.

cc @alexheretic who introduced the change

[alexheretic]

Yeah the retry loop does seem a pretty crude fix, although it's actually fairly true to real usage. It's there because we don't fully understand the cause of the race(r) condition. Iirc it only occurred in CI, I couldn't reproduce locally.

The thing is completion is pure racer, it doesn't rely on indexing. This is why completion can work before the initial RLS build. So while waiting for indexing can help here, I'd have guessed only in the same way a thread::sleep call may have.

Getting to the bottom of the failure we could try lots of travis re-running and investigation. But maybe the issue has magically disappeared anyway. Have we updated racer since then? Or maybe another dependency? We could always just merge, reverting if we see the empty completion response once again rear its ugly in CI.

[bors]

☔ The latest upstream changes (presumably #1244) made this pull request unmergeable. Please resolve the merge conflicts.

[Xanewok]

Sorry for the conflict due to translated tests. These were translated in bulk and coincidentally (I copy-pasted the wait for diagnostics -> assert on error message across every completion test) all of them now wait for initial diagnostics (so also completed indexing).

As such - maybe it might make sense to actually remove the looped requests? I'd err on the safe side and not introduce more intermittent tests since those test failures flag RLS component as unavailable in Rust repo but I remember that a specific completion test actually failed and IIRC one of those didn't didn't wait for the diagnostics, so I think we could go for it. Maybe that's the one and it might sense to remove the loop code?

@alexheretic @emilio what do you think? If that makes sense I'd very much appreciate a PR with a change to the translated completion tests in client.rs =)