Skip to content

Wait for Quick Info to render before reading content#52431

Merged
sharwell merged 1 commit intodotnet:mainfrom
sharwell:quickinfo
Apr 6, 2021
Merged

Wait for Quick Info to render before reading content#52431
sharwell merged 1 commit intodotnet:mainfrom
sharwell:quickinfo

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

@sharwell sharwell commented Apr 6, 2021

No description provided.

@sharwell sharwell requested a review from a team as a code owner April 6, 2021 16:49
@ghost ghost added the Area-Infrastructure label Apr 6, 2021
var session = broker.GetSession(view);

using var cts = new CancellationTokenSource(Helper.HangMitigatingTimeout);
while (session.State != QuickInfoSessionState.Visible)
Copy link
Copy Markdown
Contributor

@gundermanc gundermanc Apr 6, 2021

Choose a reason for hiding this comment

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

Two options on how to do this:

  • I think awaiting TriggerQuickInfoAsync() will ensure that the tooltip is active before returning the session.
  • Alternatively, if you don't care about the UI being visible, you can call GetQuickInfoItemsAsync() to just get the content that would be displayed in the tooltip.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

think awaiting TriggerQuickInfoAsync() will ensure that the tooltip is active before returning the session

We call InvokeQuickInfo before calling GetQuickInfo, and I recently updated the former to await TriggerQuickInfoAsync.

Alternatively, if you don't care about the UI being visible, you can call GetQuickInfoItemsAsync() to just get the content that would be displayed in the tooltip.

Since this is integration tests, I think we should check the actual content if possible.

Copy link
Copy Markdown
Contributor

@gundermanc gundermanc Apr 6, 2021

Choose a reason for hiding this comment

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

I'm surprised this is necessary, AFAICT we're not doing any fire-and-forget async work. It should all be tracked by the task returned by Trigger.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's exactly what I was thinking, but the log is showing test failures even though the content is clearly appearing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exceptions and screenshots can be seen in the artifacts for https://dev.azure.com/dnceng/public/_build/results?buildId=1073724&view=results

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we get a dump created at the failure point? It seems like this should be relatively straightforward for @gundermanc or somebody else to debug, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure, if you can point me to some sort of diagnostic info I can take a look opportunistically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't have a dump. Even by the time the screenshot is taken for the test failure exception you can see the expected Quick Info content.

@sharwell sharwell merged commit 5f22e19 into dotnet:main Apr 6, 2021
@ghost ghost added this to the Next milestone Apr 6, 2021
@sharwell sharwell deleted the quickinfo branch April 6, 2021 21:00
@dibarbet dibarbet modified the milestones: Next, 16.10.P3 Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants