Failing CI tests

[Xanewok]

For some time now, the CI seems to fail on tooltip tests and test_rename: https://travis-ci.org/rust-lang-nursery/rls/jobs/452854404#L1519

[nrc]

Problem is due to multiple defs from save-analysis. Working on fixing it.

[nrc]

With rust-lang/rust#55936 and the most recent changes to rls-analysis, we are now passing rename, however, we now fail deglob and find_impls :-(

[nrc]

OK, I have all tests passing locally. Needs rust-lang/rust#55936 to land and get into nightly to pass everywhere.

[norru]

Possibly root cause of #1122

[Xanewok]

On a separate note, the cmd_changing_workspace_lib_retains_bin_diagnostics test intermittently fails in Rust CI (e.g. rust-lang/rust#55934 (comment)).

What happens is there are 2 packages, a binary and a library. The test checks if, by changing the type signature in the library function, the change is propagated and binary reports an error. Unfortunately, what happens, is:

[
  {
    "jsonrpc": "2.0",
    "method": "textDocument/publishDiagnostics",
    "params": {
      "diagnostics": [
        {
          "code": "E0425",
          "message": "cannot find function `fetch_u32` in module `library`\n\nnot found in `library`",
          "range": {
            "end": {
              "character": 53,
              "line": 4
            },
            "start": {
              "character": 44,
              "line": 4
            }
          },
          "severity": 1,
          "source": "rustc"
        }
      ],
      "uri": "file:///checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/rlsit/t1/simple_workspace/binary/src/main.rs"
    }
  },
  {
    "jsonrpc": "2.0",
    "method": "textDocument/publishDiagnostics",
    "params": {
      "diagnostics": [
        {
          "code": "E0308",
          "message": "mismatched types\n\nexpected u32, found u64",
          "range": {
            "end": {
              "character": 62,
              "line": 9
            },
            "start": {
              "character": 44,
              "line": 9
            }
          },
          "severity": 1,
          "source": "rustc"
        },
        {
          "code": "unused_variables",
          "message": "unused variable: `unused`\n\nnote: #[warn(unused_variables)] on by default\nhelp: consider using `_unused` instead: `_unused`",
          "range": {
            "end": {
              "character": 30,
              "line": 2
            },
            "start": {
              "character": 24,
              "line": 2
            }
          },
          "severity": 2,
          "source": "rustc"
        }
      ],
      "uri": "file:///checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/rlsit/t1/simple_workspace/library/src/lib.rs"
    }
  }
]

We expect

          "code": "E0308",
          "message": "mismatched types\n\nexpected u32, found u64",

for the binary/src/main.rs but unfortunately we end up with

          "code": "E0425",
          "message": "cannot find function `fetch_u32` in module `library`\n\nnot found in `library`",

which is weird. I tried to reproduce it locally but couldn't.
What could cause that? Maybe there's a race condition? But we only change the return type, the fetch_u32 function should be there, either before or after the mentioned change. Spooky stuff 👻

[Xanewok]

Another case in rust-lang/rust#55947 (comment), seems like it just started happening with basically almost every PR now?

[alexheretic]

There are 4 remaining failures in the enable_tooltip_tests suite. They do seem to mostly reflect hover functionality regressions which seem to also be present in the current nightly release.

1. enum variant name change

Stable vs Nightly

Stable

Nightly

2. enum variant missing hover

Stable vs Nightly

Stable

Nightly

3. Vec::<u32>::default() missing hover

Stable vs Nightly

Stable

Nightly

4. More module hovers include a link

This one at least can just be updated as it isn't a regression.

Stable vs Nightly

Stable

Nightly

[alexheretic]

Cool these 4 seem to be addressed now, nice one @Xanewok!

[Xanewok]

paths/variants were fixed by rust-lang/rust#56060 so that would be @nrc's doing :)

What's worrying is that we have macOS-specific tooltip failures and they seem to like we're not able to show type definitions in those tooltips.

I don't have access to a macOS machine, so I can't really dig into that - if some of you have, could you verify some of these failures and see if there's a problem with type definition on hover in general?

[alexheretic]

Now we have 1 hover test failing
expected: { language: "rust", value: "liballoc/string.rs" }, actual: { language: "rust", value: "src/liballoc/string.rs" }

I haven't been following the upstream change closely enough to know which one is best. Maybe we can just update the test?

Update: I got the expected/actual the wrong way round the first time.

[Xanewok]

Yeah we should - it’s due to rust-lang/rust#53586.

[alexheretic]

Actually I got expected/actual reversed there, fixed now. So I think the test is correct, and we shouldn't be seeing the src/ prefix. Is that right?

[Xanewok]

Fixed that in #1155.

[aloucks]

I don't believe the src prefix used to show up before.

[alexheretic]

We're almost there now with Windows builds passing. But it looks like the mac tooltip tests are a bit flaky, they sometimes pass, sometimes fail.

[Xanewok]

In the https://travis-ci.org/rust-lang/rls/jobs/465961072 (#1152) failure it looks like we get type definition from the Racer fallback but we expect the rls-analysis info.

We should only execute the fallback when we don't have the type info, correct?

[alexheretic]

Yep it should be fallback on error.

rls/src/actions/hover.rs

Lines 899 to 913 in c84b8ad

	let hover_span_def = hover_span_def.or_else(|e| {
	debug!(
	"tooltip: racer_fallback_enabled: {}",
	racer_fallback_enabled
	);
	if racer_fallback_enabled {
	debug!("tooltip: span_def is empty, attempting with racer");
	racer_def(&ctx, &hover_span).ok_or_else(|| {
	debug!("tooltip: racer returned an empty result");
	e
	})
	} else {
	Err(e)
	}
	});

This is why it would perhaps be clearer to have tests that are for analysis only, in these we'll disable racer_completion. Additionally we could have racer ones, that are known to not work from analysis maybe.

Right now it's hard to know which bit is meant to work.

[Xanewok]

Rest of tests work correctly now, closing this in favor of #1151 since that test needs some work and fails intermittently on macOS.