Add integration tests to ensure that error context data is being sent to clients (both LSP and legacy protocol)

[stereotype441]

(Parent issue #44897)

As of a42244f, the analyzer now generates context messages in some circumstances explaining why type promotion failed. We should ensure that we have adequate integration tests to verify that these context messages are being delivered to analysis server clients. We should test both the LSP protocol (used by VSCode) and the legacy protocol (used by IntelliJ).

Related: #44901, #44902.

[stereotype441]

I'm not sure who to assign this to--perhaps @bwilkerson?

[stereotype441]

@devoncarew do you think anyone will have a chance to work on this before the branch cut?

[devoncarew]

Do we not already have coverage here? This isn't about testing for the presence of the messages in general - that's covered by #45068 and the test.py work (#44905).

This is just to make sure that we're populating the information in the protocol messages to IDEs? @bwilkerson / @DanTup - is this already covered by existing tests?

[DanTup]

There's a test in LSP here:

sdk/pkg/analysis_server/test/lsp/diagnostic_test.dart

Lines 129 to 144 in 3ce1969

	Future<void> test_contextMessage() async {
	newFile(mainFilePath, content: '''
	void f() {
	x = 0;
	int x;
	print(x);
	}
	''');
	final diagnosticsUpdate = waitForDiagnostics(mainFileUri);
	await initialize();
	final diagnostics = await diagnosticsUpdate;
	expect(diagnostics, hasLength(1));
	final diagnostic = diagnostics.first;
	expect(diagnostic.relatedInformation, hasLength(1));
	}

It's not currently testing the specific text/location (it may be worth extending to do that), though it is ensuring there is a relatedInformation (which is where the context message shows up if I understand correctly).

[bwilkerson]

I don't think we have an "integration" test for the legacy protocol (where "integration" here means end-to-end). We do have tests to ensure that we correctly convert analysis errors from the analyzer into the protocol object representing analysis errors (https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/test/utilities/analyzer_converter_test.dart#L98).

[stereotype441]

@bwilkerson I'm not sure what you mean by "end-to-end", but we do have several tests in pkg/analysis_server/test/integration/ that call themselves integration tests; those are the kinds of tests I was thinking of.

I think it would be an adequate test to make a test similar to pkg/analysis_server/test/integration/analysis/error_test.dart, but where the error being tested has context information, so that we can make sure that the context information is included in the information the analysis server sends back to its client.

[bwilkerson]

... tests in pkg/analysis_server/test/integration/ that call themselves integration tests ...

Yep, those are what I was talking about as well.

[DanTup]

I missed the "integration" part. I've opened https://dart-review.googlesource.com/c/sdk/+/193800/ that adds an LSP integration test that checks the contexts come through (unlike the test I noted above, these spawn the server in another process and then communicate with it exactly as a client would - although the in-process tests for the LSP server exercise essentially the same stack including the full protocol classes, just without the process boundary and using an in-memory filesystem).