Skip to content

Disable TypingHelpDirectiveWorks for legacy completion#36190

Merged
sharwell merged 1 commit intodotnet:masterfrom
sharwell:disable-flaky-test
Jun 6, 2019
Merged

Disable TypingHelpDirectiveWorks for legacy completion#36190
sharwell merged 1 commit intodotnet:masterfrom
sharwell:disable-flaky-test

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

@sharwell sharwell commented Jun 5, 2019 •
edited
Loading

This test is flaky in legacy completion, but legacy completion is slated for removal so the test failures were not fully investigated.

This change addresses failures of this test with the following message:

System.Exception : Unable to find expected content in REPL within 10000 milliseconds and no exceptions were thrown. Actual content:\r\n[[Resetting execution engine.\r\nLoading context from 'CSharpInteractive.rsp'.]]

Server stack trace: 
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InteractiveWindow_InProc.WaitForPredicate(Func`1 getValue, Func`2 isExpectedValue)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InteractiveWindow_InProc.WaitForLastReplOutputContains(String outputText)
   at System.Runtime.Remoting.Messaging.StackBuilderSink._PrivateProcessMessage(IntPtr md, Object[] args, Object server, Object[]& outArgs)
   at System.Runtime.Remoting.Messaging.StackBuilderSink.SyncProcessMessage(IMessage msg)

Exception rethrown at [0]: 
   at System.Runtime.Remoting.Proxies.RealProxy.HandleReturnMessage(IMessage reqMsg, IMessage retMsg)
   at System.Runtime.Remoting.Proxies.RealProxy.PrivateInvoke(MessageData& msgData, Int32 type)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InteractiveWindow_InProc.WaitForLastReplOutputContains(String outputText)
   at Roslyn.VisualStudio.IntegrationTests.CSharp.CSharpInteractive.TypingHelpDirectiveWorks() in /_/src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpInteractive.cs:line 101

During the investigation, I observed that each of the failure cases was preceded by an exception thrown in ChainTaskAndNotifyControllerWhenFinished in the awaiter's GetResult on this line:

roslyn/src/EditorFeatures/Core/Implementation/IntelliSense/ModelComputation.cs

Line 150 in c78f1e5

await ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(alwaysYield: true, _stopCancellationToken);

The failure does not appear to be caused by an unhandled exception, but rather the exception appears to be an observable symptom of a race condition which occurs only when this test is going to fail.

@sharwell
Disable TypingHelpDirectiveWorks for legacy completion
4906000 
This test is flaky in legacy completion, but legacy completion is slated
for removal so the test failures were not fully investigated.
@sharwell sharwell requested a review from a team as a code owner June 5, 2019 22:20
@sharwell sharwell added Area-Infrastructure Integration-Test Test failures in roslyn-integration-CI labels Jun 5, 2019
@sharwell sharwell mentioned this pull request Jun 5, 2019
Merged
jasonmalinowski
jasonmalinowski approved these changes Jun 5, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the investigation, I observed that each of the failure cases was preceded by an exception thrown in ChainTaskAndNotifyControllerWhenFinished in the awaiter's GetResult on this line:

Thanks for doing this investigation. I was about to ask "why are we sure this isn't a product bug?" but if the answer is "yes, it seems to be a product bug in code we're not maintaining", great!

@sharwell
Copy link
Copy Markdown
Contributor Author

sharwell commented Jun 5, 2019

@jasonmalinowski Yeah, I narrowed it down to legacy completion scenarios fairly quickly. Then I did a time-boxed investigation to see if I could correct it rather than skip it. I wasn't able to find a working solution in a reasonable amount of time, so I included what I found out so far and proceeded with a targeted skip.

@sharwell sharwell merged commit eadc22b into dotnet:master Jun 6, 2019
@sharwell sharwell deleted the disable-flaky-test branch June 6, 2019 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tmat tmat tmat approved these changes

@jasonmalinowski jasonmalinowski jasonmalinowski approved these changes

Assignees

No one assigned

Labels

Area-Infrastructure Integration-Test Test failures in roslyn-integration-CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sharwell @tmat @jasonmalinowski