Integration fixes by sharwell · Pull Request #70635 · dotnet/roslyn

sharwell · 2023-10-31T04:40:04Z

Submitting to CI to catch cases I might have missed during the port. Will review and rebase with updates before marking reading for review.

Closes #20371
Closes #25814
Closes #35965
Closes #36763
Closes #37689
Closes #38301
Closes #40160
Closes #45234
Closes #46027
Closes #57423
Closes #70582

CyrusNajmabadi

Awesome!

sharwell · 2023-11-03T14:20:44Z

src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/EditorInProcess.cs

-            object? obj = null;
-            shell.PostExecCommand(cmdGroup, (uint)cmdID, (uint)cmdExecOpt, ref obj);
+            // It's not known why WaitForCompletionSetAsync fails to stabilize calls to IsCompletionActive.
+            await Task.Delay(TimeSpan.FromSeconds(1));


📝 The most flaky tests in this pull request are all failing in calls that assert the result of IsCompletionActiveAsync. We should work with editor team to make this a specific wait instead of an arbitrary constant.

Closes dotnet#20371

Closes dotnet#25814

Closes dotnet#35965 Closes dotnet#36763 Closes dotnet#37689

Closes dotnet#38301 Closes dotnet#45234 Closes dotnet#70582

Closes dotnet#57423

Related to dotnet#35965 Related to dotnet#19526

Related to dotnet#38301

Consistently restore state to a specific environment at the start of the test, and condition the expected output based on that environment. Possibly related to dotnet#57701

sharwell · 2023-11-03T16:22:30Z

...lStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowInProcessExtensions.cs

+            // It's not known why WaitForCompletionSetAsync fails to stabilize calls to GetCompletionItemsAsync.
+            await Task.Delay(TimeSpan.FromSeconds(1));


📝 Added a delay here as well. It looks like ICompletionBroker is introducing significant instability in the overall testing. We should work with the editor to stabilize these APIs.

yea, we already switched to another API based on Kayle's suggestion, but it didn't help
https://github.com/dotnet/roslyn/pull/70578/files

genlu · 2023-11-03T18:15:20Z

        options.SetOptionValue(DefaultOptions.ResponsiveCompletionOptionId, false);

There's also this option in editor, we should check with Kayle to see if we need to set it too

update: nvm, the default is false

dibarbet

A couple random nits, feel free to take or ignore. I generally only skimmed the actual test files. Nothing seemed egregiously off!

dibarbet · 2023-11-03T20:58:28Z

src/VisualStudio/IntegrationTest/New.IntegrationTests/AbstractIntegrationTest.cs

+                var dte = await TestServices.Shell.GetRequiredGlobalServiceAsync<SDTE, EnvDTE.DTE>(HangMitigatingCancellationToken);
+                if (dte.Debugger.CurrentMode != EnvDTE.dbgDebugMode.dbgDesignMode)
+                {
+                    dte.Debugger.TerminateAll();


Is this here to workaround an issue with the debugger? Is there some state thats not clearing after a previous test?

If a test fails during a debugging session, it will not stop the debugger before exiting the test method. This method cleans up from whatever state the test completed in so the failure is isolated to just the test with the assertion failure.

dibarbet · 2023-11-03T21:03:57Z

...isualStudio/IntegrationTest/New.IntegrationTests/InProcess/ChangeSignatureDialogInProcess.cs

+            while (true)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                if (await TryGetDialogAsync(cancellationToken) is null)


there's a TryWaitForAsync helper in one of the other services below - could be used here too.

Maybe. I'm not thrilled with the current behavior but it matches the old code so left it for now.

dibarbet · 2023-11-03T21:08:42Z

src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/GenerateTypeDialogInProcess.cs

+            while (true)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                if (await TryGetDialogAsync(cancellationToken) is null)


Another place where we could use the helper (there's more further down as well, but I won't comment them all)

Yeah, every dialog helper has the same code.

dibarbet · 2023-11-03T21:10:17Z

src/VisualStudio/IntegrationTest/IntegrationTests/AbstractEditorTest.cs

-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-#nullable disable


Can we remove the old integration test project entirely at this point?

Yes I'll do that in the coming week. There is a bunch of support code in RunTests and the build scripts as well.

ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 31, 2023

CyrusNajmabadi reviewed Oct 31, 2023

View reviewed changes

build-analysis bot mentioned this pull request Nov 1, 2023

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

sharwell commented Nov 3, 2023

View reviewed changes

sharwell added 25 commits November 3, 2023 09:24

Port keyword highlighting tests to the new framework

21ad298

Port outlining tests to the new framework

dc108a1

Port reference highlighting tests to the new framework

6c8cfce

Port code actions tests to the new framework

3c5ec91

Closes dotnet#20371

Port organizing tests to the new framework

4beeb05

Port UI Context tests to the new framework

72f537a

Remove unused constants

d40dde5

Port code definition window tests to the new framework

979443e

Port BasicEndConstruct to the new framework

eaf0b25

Return full caret position information from GetCaretPositionAsync

25e87ab

Port immediate window tests to the new framework

6eabb74

Closes dotnet#25814

Port edit and continue tests to the new framework

e5442be

Closes dotnet#35965 Closes dotnet#36763 Closes dotnet#37689

Update ThrowingTraceListener integration

77b157f

Port complete statement tests to the new framework

26be7b8

Port IntelliSense tests to the new framework

06c0963

Closes dotnet#38301 Closes dotnet#45234 Closes dotnet#70582

Log pending asynchronous operations on test failure

5bbb469

Enable GFUFuzzyMatchAfterRenameTrackingAndAfterGenerateType

a19030f

Closes dotnet#57423

Port signature help tests to the new framework

7cdb697

Port expression evaluator tests to the new framework

17fa034

Related to dotnet#35965 Related to dotnet#19526

Port extract method tests to the new framework

29da61d

Port extract interface tests to the new framework

fc6b8f0

Port change signature tests to the new framework

5cf3f83

Use WellKnownCommands.Edit.Undo for clarity

af2fbec

Avoid JoinableTaskFactory.Run in async methods

f2e0762

Fix infinite loop trying to close windows

986e4f8

sharwell added 12 commits November 3, 2023 09:24

Port generate type tests to the new framework

3c78416

Port generate constructor tests to the new framework

a17c296

Port CSharpReplIdeFeatures to the new test framework

2666663

Port generate equals and hash code tests to the new framework

feddd03

Port WinForms tests to the new framework

2dc5302

Port interactive tests to the new framework

006f074

Port rename file tests to new framewosrk

dd2a7cb

Port move to namespace tests to the new framework

7ff1f86

Fix and enable CPSProject_GeneralPropertyGroupUpdated

a9e5f60

Related to dotnet#38301

Wait for rename before verifying rename tags

8ff23ef

Stabilize test WorkspaceClearedAfterReset

cdf80f7

Consistently restore state to a specific environment at the start of the test, and condition the expected output based on that environment. Possibly related to dotnet#57701

Avoid cluttering logs with known assertion failure

f480d17

sharwell force-pushed the integration-fixes branch from 9f2fe67 to 74453d3 Compare November 3, 2023 14:25

sharwell marked this pull request as ready for review November 3, 2023 14:25

sharwell requested review from a team as code owners November 3, 2023 14:25

Add a delay to try and stabilize completion tests

9075f85

sharwell force-pushed the integration-fixes branch from 74453d3 to 9075f85 Compare November 3, 2023 16:21

sharwell commented Nov 3, 2023

View reviewed changes

dibarbet approved these changes Nov 3, 2023

View reviewed changes

sharwell merged commit 23ddf9f into dotnet:main Nov 4, 2023

ghost added this to the Next milestone Nov 4, 2023

sharwell deleted the integration-fixes branch November 4, 2023 01:44

allisonchou mentioned this pull request Nov 9, 2023

[Automated] PRs inserted in VS build main-34309.104 #70754

Closed

RikkiGibson modified the milestones: Next, 17.9 P2 Nov 28, 2023

		// It's not known why WaitForCompletionSetAsync fails to stabilize calls to GetCompletionItemsAsync.
		await Task.Delay(TimeSpan.FromSeconds(1));

Conversation

sharwell commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

genlu commented Nov 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sharwell commented Oct 31, 2023 •

edited

Loading

genlu commented Nov 3, 2023 •

edited

Loading