Integration tests: BoxSelection, CSharpInteractiveDirectives, CSharpInteractiveFormsAndWpf, CSharpReplClassification #18137

ivanbasov · 2017-03-24T00:08:31Z

@dotnet/roslyn-ide

…onTests

…inal

ivanbasov · 2017-03-27T16:52:16Z

Ping. Please review. #Closed

jmarolf

Looks like things were merged incorrectly

jmarolf · 2017-03-28T17:12:24Z

src/VisualStudio/IntegrationTest/IntegrationTests/AbstractEditorTest.cs

@@ -78,5 +73,258 @@ protected void SetUpEditor(string markupCode)
                VisualStudioWorkspaceOutOfProc.SetPrettyListing(LanguageName, originalValue);
            }
        }
+


this looks like a bad marge. These should be in extension methods #Resolved

Thank you! Removed all methods below SetUpEditor because they are already in extension methods. #Closed

jmarolf · 2017-03-28T17:13:03Z

src/VisualStudio/IntegrationTest/IntegrationTests/AbstractIntegrationTest.cs

@@ -47,5 +51,140 @@ protected KeyPress Shift(VirtualKey virtualKey)

        protected KeyPress Alt(VirtualKey virtualKey)
            => new KeyPress(virtualKey, ShiftState.Alt);
+


this looks like a bad marge. These should be in extension methods #Resolved

Thank you! Removed everything below the Alt method and merged new callers. #Closed

jasonmalinowski

Many comments so far. There are many tests that still have commented out bodies that make me thing this isn't quite ready for review yet. You'll also need to chat with @jmarolf as we just discussed how we might want to be refactoring some stuff.

jasonmalinowski · 2017-03-28T18:01:00Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/BoxSelection.cs

+namespace Roslyn.VisualStudio.IntegrationTests.CSharp
+{
+    [Collection(nameof(SharedIntegrationHostFixture))]
+    public class BoxSelection : AbstractInteractiveWindowTest


If this is all interactive window testing, call it InteractiveWindowBoxSelection? #Resolved

Agree. Thanks! #Closed

jasonmalinowski · 2017-03-28T18:02:45Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpInteractiveCommands.cs

-            this.ExecuteCommand("Edit.LineDown");
-            this.ExecuteCommand("Edit.Paste");
-            this.ExecuteCommand("Edit.Paste");
+            ExecuteCommand(WellKnownCommandNames.Edit_LineStart);


This series of command executions seems really fishy. What is this test actually asserting? #Resolved

There can be differences in commands available between the editor and the interactive.
The test verifies that commands mentioned are available in the interactive window. #Closed

Does it though? If every command silently fails, this test passes? #Resolved

Thank you! I see your point.

If it fails silently, then we do not catch it here. We get an error the following cases:

If a command is not available

If a command failed and threw an exception

This is the scope of the original test.

Let me add couple more verification in the end here. #Closed

jasonmalinowski · 2017-03-28T18:05:01Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpInteractiveDirectives.cs

+        {
+            SubmitText("#foo");
+            this.WaitForLastReplOutput("(1,2): error CS1024: Preprocessor directive expected");
+            // TODO implement GetErrorListErrorCount: https://github.com/dotnet/roslyn/issues/18035


The TODO confuses me -- is this a new TODO or something that's being ported? #ByDesign

There is a difference between error count in the interactive and in the editor. Error count in editor is displayed in the errors windows and we can build and then see errors there. Editors tests follow this pattern.

However, as far as I can see, we cannot follow this patter for interactive solution. We should find errors in the interactive (scripting) window, I could not find an easy way to do so. I would prefer to solve this problem with a separate code review. That is why I created a new issue and added TODOs for it. I think that the priority of this verification is low because we already verify error messages within those tests.

Please let me know if you prefer to arrange this other way. #Closed

jasonmalinowski · 2017-03-28T18:05:32Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpInteractiveDirectives.cs

+            this.WaitForLastReplOutputContains("CS7010: Quoted file name expected");
+        }
+
+//        [Fact]


Why a commented out test to start? #Resolved

jasonmalinowski · 2017-03-28T18:06:04Z

src/VisualStudio/IntegrationTest/TestUtilities/WellKnowTagNames.cs

+
+namespace Microsoft.VisualStudio.IntegrationTest.Utilities
+{
+    public class WellKnowTagNames


"Known". #Resolved

Thank you! #Closed

jasonmalinowski · 2017-03-28T18:12:46Z

src/VisualStudio/IntegrationTest/IntegrationTests/AbstractEditorTest.cs

+            }
+        }
+
+        protected void VerifyTextDoesNotContain(string expectedText)


I think we're trying to get rid of "verify methods that just call assert in the most trivial of ways". #Resolved

not sure I understand the comment. just verified that this method is not used and was removed. Likely, @jmarolf already removed it before and I restored it by a bad merge. Removed it now. #Closed

jasonmalinowski · 2017-03-28T18:13:00Z

src/VisualStudio/IntegrationTest/IntegrationTests/AbstractEditorTest.cs

+            Assert.DoesNotContain(expectedText, editorText);
+        }
+
+        protected void VerifyCompletionItemDoesNotExist(params string[] expectedItems)


VerifyCompletionItemsDoNotExist. #Resolved

Thank you! #Closed

jasonmalinowski · 2017-03-28T18:13:59Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpInteractiveDirectives.cs

+
+namespace Roslyn.VisualStudio.IntegrationTests.CSharp
+{
+


Blank line. #Resolved

Thanks! #Closed

jasonmalinowski · 2017-03-28T18:15:19Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpReplIdeFeatures.cs

+            base.Dispose();
+        }
+
+        // https://github.com/dotnet/roslyn/issues/801


Use Skip to track the test, don't comment out Facts ever. #Resolved

Makes sense. Thank you! #Closed

jasonmalinowski · 2017-03-28T18:15:33Z

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpReplIdeFeatures.cs

+        public void VerifyCodeActionsAvailableInCurrentSubmission()
+        {
+            InsertCode("Directory.Exists(\"foo\");");
+            //      <VerifyCodeActions ApplyFix = "using System.IO;" >


Hmm? #Resolved

Code Actions are available in Interactive. If one opens Visual Studio, actions described in the original test can be performed.

However, the Visual Studio instance we use for tests does not support these code actions (set a break point during an integration test to see the difference).

So, I migrated this test (that was commented out in the original Tao test set) but have to keep it disabled. Please let me know if you see a better solution for this. #Closed

I thought we had disabled code actions in interactive, which might be why this test was commented out. You'll probably have to spelunk through source history to figure out what happened.

In any case my bigger confusion was "commented out random XML with no explanation of where it came from" will definitely get a "hmm?" from me. #Resolved

Thank you!

I migrated some more disabled tests.

There are 2 tests with "XML". Both seem to be reasonable to keep but not reasonable to migrate right now.

Now I migrated all tests with CodeActions. However, they are disabled due to open issues. I can verify CodeActions scenarios in standalone VisualStudio but not in integration tests. Will sync with Jonathon tomorrow.

In reply to: 108558613 [](ancestors = 108558613)

@jmarolf

Dismiss since I'm going on vacation but @jmarolf can take it from here.

ivanbasov and others added 30 commits March 13, 2017 16:43

CSharpInteractiveCommands

5e7aa67

CSharpInteractiveCommands

06a6371

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

9ffd288

…onTests

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

f7a5c53

…onTests

Interactive Commands tests

1764c30

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

4f4aa7f

…onTests

Interactive Commands tests

6383c45

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

103153b

…onTests

Code review feedback

7165eae

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

8994583

…onTests

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

127b71e

…onTests

REPL Classification and REPL Intellisence tests

f852cea

Minor refactoring of OutOfProc

7c2676a

Box selection and interactive directives

bbbc25b

Merge remote-tracking branch 'origin/master' into ivanbasov/Integrati…

4c3dfdf

…onTests

code review feedback

ecc9ace

Merge branch 'master' into ivanbasov/IntegrationTests

c91e92a

fixing merge conflict

7f5c264

merge from dotnet to fork

0cae52b

interactive tests for forms and WPF

4c68c4c

Merge remote-tracking branch 'upstream/master' into MoreIntegrationTests

f4d236c

CSharpSendToInteractive draft

6556354

Merge remote-tracking branch 'upstream/master' into MoreIntegrationTests

bb77733

CSharpSendToInteractive updated

ffb939e

merge from upstream/master

cfe5469

resolve merge conflicts

4bec9c0

merge from parent

af63585

Wait for pattern

f8ed530

merge from MoreInstreationTests

8b19aa7

all tests pass

796b296

ivanbasov requested review from tmat, tannergooding and jinujoseph March 24, 2017 00:08

dnfclas added the cla-already-signed label Mar 24, 2017

ivanbasov added 2 commits March 23, 2017 18:08

Increasing timeout for build and reset from project operation

ea94bcb

Merge remote-tracking branch 'upstream/master' into IntegrationTestsF…

2a25098

…inal

jasonmalinowski self-requested a review March 24, 2017 18:32

tests for repl ide features

3d4b480

ivanbasov added 7 commits March 27, 2017 11:21

tests for repl ide features

e64e4cd

Merge remote-tracking branch 'upstream/master' into MoreIntegrationTests

0bbd946

Merge branch 'IntegrationTestsFinal' into MoreIntegrationTests

3780c72

removing VerifyCodeActions method already moved to a parent class

93f9d90

merge from upstream

05eb9e0

Merge remote-tracking branch 'upstream/master' into MoreIntegrationTests

8bb0a32

refactoring: moving methods from abstract classes to extension methods

1b73aea

ivanbasov requested a review from jmarolf March 28, 2017 01:06

minor cleanup

6687681

jmarolf suggested changes Mar 28, 2017

View reviewed changes

jasonmalinowski previously requested changes Mar 28, 2017

View reviewed changes

ivanbasov requested a review from dpoeschl March 28, 2017 20:23

Code review feedback

a77b673

ivanbasov added 2 commits March 28, 2017 16:58

merge from upstream

6202c90

fixing flakey test

577db00

jmarolf approved these changes Mar 29, 2017

View reviewed changes

migrating (disabled) code action tests

38b511c

ivanbasov merged commit 85b4b11 into dotnet:master Mar 29, 2017

ivanbasov deleted the MoreIntegrationTests branch March 29, 2017 02:11

ivanbasov mentioned this pull request Mar 29, 2017

Reenable Interactive Window Integration Tests #14369

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration tests: BoxSelection, CSharpInteractiveDirectives, CSharpInteractiveFormsAndWpf, CSharpReplClassification #18137

Integration tests: BoxSelection, CSharpInteractiveDirectives, CSharpInteractiveFormsAndWpf, CSharpReplClassification #18137

ivanbasov commented Mar 24, 2017

ivanbasov commented Mar 27, 2017 •

edited

Loading

jmarolf left a comment

jmarolf Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jmarolf Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski left a comment

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 29, 2017

		@@ -47,5 +51,140 @@ protected KeyPress Shift(VirtualKey virtualKey)

		protected KeyPress Alt(VirtualKey virtualKey)
		=> new KeyPress(virtualKey, ShiftState.Alt);


		namespace Roslyn.VisualStudio.IntegrationTests.CSharp
		{

Integration tests: BoxSelection, CSharpInteractiveDirectives, CSharpInteractiveFormsAndWpf, CSharpReplClassification #18137

Integration tests: BoxSelection, CSharpInteractiveDirectives, CSharpInteractiveFormsAndWpf, CSharpReplClassification #18137

Conversation

ivanbasov commented Mar 24, 2017

ivanbasov commented Mar 27, 2017 • edited Loading

jmarolf left a comment

Choose a reason for hiding this comment

jmarolf Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jmarolf Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski left a comment

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski Mar 28, 2017 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 29, 2017

Choose a reason for hiding this comment

ivanbasov commented Mar 27, 2017 •

edited

Loading

jmarolf Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jmarolf Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading

ivanbasov Mar 28, 2017 •

edited

Loading

jasonmalinowski Mar 28, 2017 •

edited by ivanbasov

Loading