Create new test for covering Refactoring ->Rename feature for DotNet LS by musienko-maxim · Pull Request #11006 · eclipse-che/che

musienko-maxim · 2018-08-31T08:59:59Z

What does this PR do?

Implement functional test for checking renaming class in DotNet projects.
Add new simple (hello world) DotNet project to project templates
Add new methods and locators for performing renaming with Cshrp LS.

What issues does this PR fix or reference?

#10195
@dmytro-ndp, @Ohrimenko1988, @SkorikSergey

SkorikSergey · 2018-09-03T13:13:36Z

...um-test/src/test/java/org/eclipse/che/selenium/languageserver/Csharp/CSharpRenamingTest.java

+  public void checkRenaming() {
+    String newClassName = "HelloWorld";
+    String textFragmentAfterRenaming = "public class HelloWorld";
+    projectExplorer.openItemByPath(PROJECT_NAME);


This project was already opened in setUp() method(line 71).

SkorikSergey · 2018-09-03T13:18:51Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

+   *
+   * @param renamevalue
+   */
+  public void doRenamingByLanguageServerField(String renamevalue) {


It maybe better to rename variable to 'renameValue'

SkorikSergey · 2018-09-03T13:21:17Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

  }
+
+  /**
+   * wait renaming field in the Editor (usually it field is used by language servers) type new


Maybe there need a comma after brackets

Ohrimenko1988 · 2018-09-03T13:21:34Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

+   * @param renamevalue
+   */
+  public void doRenamingByLanguageServerField(String renamevalue) {
+    seleniumWebDriverHelper


Please use "SeleniumWebDriverHelper#setText(WebElement webElement, String value)"

dmytro-ndp · 2018-09-03T13:28:10Z

...um-test/src/test/java/org/eclipse/che/selenium/languageserver/Csharp/CSharpRenamingTest.java

+      editor.waitTextIntoEditor(textFragmentAfterRenaming);
+    } catch (TimeoutException ex) {
+      // remove try-catch block after issue has been resolved
+      fail("Known issue https://github.com/eclipse/che/issues/10180", ex);


Is it permanent failure? If so, it's better to express it in error message like the follow:

Known permanent failure https://github.com/eclipse/che/issues/10180

dmytro-ndp · 2018-09-03T13:39:55Z

...um-test/src/test/java/org/eclipse/che/selenium/languageserver/Csharp/CSharpRenamingTest.java

+
+  @BeforeClass
+  public void setUp() throws Exception {
+    URL resource = getClass().getResource("/projects/CsharpHelloWorld");


To be consistent: CsharpHelloWorld > CSharpHelloWorld

dmytro-ndp · 2018-09-03T13:41:35Z

...test/src/test/java/org/eclipse/che/selenium/languageserver/Csharp/CSharpFileEditingTest.java

 *   Red Hat, Inc. - initial API and implementation
 */
-package org.eclipse.che.selenium.languageserver;
+package org.eclipse.che.selenium.languageserver.Csharp;


package name should be in lower case: Csharp > csharp

dmytro-ndp · 2018-09-03T16:30:06Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

 import static org.eclipse.che.selenium.pageobject.CodenvyEditor.Locators.IMPLEMENTATION_CONTAINER;
 import static org.eclipse.che.selenium.pageobject.CodenvyEditor.Locators.ITEM_TAB_LIST;
 import static org.eclipse.che.selenium.pageobject.CodenvyEditor.Locators.JAVA_DOC_POPUP;
+import static org.eclipse.che.selenium.pageobject.CodenvyEditor.Locators.LANGUAGE_SERVER_RENFACTORING_RENAME_FIELD_CSS;


typo RENFACTORING

dmytro-ndp · 2018-09-03T16:30:29Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

    String HOVER_POPUP_XPATH =
        "//div[@class='textviewTooltip' and contains(@style,'visibility: visible')]";
    String AUTOCOMPLETE_PROPOSAL_DOC_ID = "gwt-debug-content-assistant-doc-popup";
+    String LANGUAGE_SERVER_RENFACTORING_RENAME_FIELD_CSS = "input.orionCodenvy";


typo RENFACTORING

dmytro-ndp · 2018-09-03T16:31:24Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

  }
+
+  /**
+   * wait renaming field in the Editor (usually it field is used by language servers), type new


type new rename value > type new value

dmytro-ndp · 2018-09-03T16:32:48Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

  }
+
+  /**
+   * wait renaming field in the Editor (usually it field is used by language servers), type new


What does Language Server Field mean? Did you mean Code Assistant?

Actually we have 2 UI part for renaming:

For java

For other languages

I see, thank you for the explanation.

dmytro-ndp · 2018-09-03T16:33:28Z

selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/CodenvyEditor.java

    String HOVER_POPUP_XPATH =
        "//div[@class='textviewTooltip' and contains(@style,'visibility: visible')]";
    String AUTOCOMPLETE_PROPOSAL_DOC_ID = "gwt-debug-content-assistant-doc-popup";
+    String LANGUAGE_SERVER_RENFACTORING_RENAME_FIELD_CSS = "input.orionCodenvy";


input.orionCodenvy style name looks too general IMHO

I agree but it is part of Orion component. With this general name of css locator is simple and reliable. If you have other idea for this - we will apply it

dmytro-ndp · 2018-09-04T12:03:24Z

...um-test/src/test/java/org/eclipse/che/selenium/languageserver/csharp/CSharpRenamingTest.java

+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class CSharpRenamingTest {


According to name convention there should be CSharpClassRenamingTest test class name, IMHO.

dmytro-ndp · 2018-09-04T12:24:37Z

I see you created separate test class to test C# renaming, but it consumes additional time for creation and starting test workspace, and for resolving #C project dependencies.
How about testing renaming by using CSharpFileEditingTest.java to save time?

musienko-maxim · 2018-09-04T12:47:36Z

I totally agree with you but at this moment C# language is very unstable. During the test we can get an error that was described in the issue: #10151. This one totally breaks the test environment. And all tests after that will be broken. In this case we cannot track if the known issue has been fixed or not (because the workspace does not work properly). Also we won't know which feature works, because all tests related to the workspace will fail anyway. So it is temporary solution i think after we stabilize and fix known issues, we can join this into common class

dmytro-ndp · 2018-09-04T13:34:40Z

Got it! Thank you for response.

musienko-maxim · 2018-09-04T13:54:50Z

ci-build

musienko-maxim added 5 commits August 16, 2018 19:10

create sceleton for hello world C# project

9050122

resolve conflicts

959227f

Merge branch 'master' into CHE#10195

15e8837

Merge branch 'master' into CHE#10195

d75fa0d

Implement test for checking renaming by Csharp language server test

6f7606a

musienko-maxim requested review from Ohrimenko1988 and dmytro-ndp as code owners August 31, 2018 09:00

add empty line

8eb6c9f

musienko-maxim changed the title ~~Create new test for covering Refactoring ->Rename feature for DotNet LS~~ [WIP] Create new test for covering Refactoring ->Rename feature for DotNet LS Aug 31, 2018

musienko-maxim added the status/in-progress This issue has been taken by an engineer and is under active development. label Aug 31, 2018

musienko-maxim requested a review from SkorikSergey August 31, 2018 09:03

benoitf added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Aug 31, 2018

musienko-maxim added 5 commits September 3, 2018 15:25

fix typo

bacb212

Add license headers to the files

4cb2fdd

Merge branch 'CHE#10195' of github.com:eclipse/che into CHE#10195

6e5282b

correct formatting

a682f48

add package for Csharm tests, add test to the suite

28cea63

musienko-maxim changed the title ~~[WIP] Create new test for covering Refactoring ->Rename feature for DotNet LS~~ Create new test for covering Refactoring ->Rename feature for DotNet LS Sep 3, 2018

SkorikSergey reviewed Sep 3, 2018

View reviewed changes

Ohrimenko1988 reviewed Sep 3, 2018

View reviewed changes

Ohrimenko1988 approved these changes Sep 3, 2018

View reviewed changes

dmytro-ndp reviewed Sep 3, 2018

View reviewed changes

musienko-maxim added 2 commits September 3, 2018 16:37

add proposals to the PR

761e62c

remove duplicated step

ee6411a

dmytro-ndp reviewed Sep 3, 2018

View reviewed changes

apply consistent names for importing and resource

214af56

SkorikSergey approved these changes Sep 3, 2018

View reviewed changes

dmytro-ndp reviewed Sep 3, 2018

View reviewed changes

musienko-maxim added 2 commits September 4, 2018 11:15

fix typo

cadfc8b

apply formatting

6197e52

dmytro-ndp reviewed Sep 4, 2018

View reviewed changes

rename class for more readability

e6f4066

dmytro-ndp approved these changes Sep 4, 2018

View reviewed changes

musienko-maxim merged commit 27075e5 into master Sep 4, 2018

musienko-maxim deleted the CHE#10195 branch September 4, 2018 15:43

benoitf added this to the 6.11.0 milestone Sep 4, 2018

Conversation

musienko-maxim commented Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

What issues does this PR fix or reference?

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!

dmytro-ndp Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmytro-ndp Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmytro-ndp Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmytro-ndp Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musienko-maxim Sep 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

dmytro-ndp Sep 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmytro-ndp commented Sep 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

musienko-maxim commented Sep 4, 2018

Uh oh!

dmytro-ndp commented Sep 4, 2018

Uh oh!

musienko-maxim commented Sep 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

musienko-maxim commented Aug 31, 2018 •

edited

Loading

dmytro-ndp Sep 3, 2018 •

edited

Loading

dmytro-ndp Sep 3, 2018 •

edited

Loading

dmytro-ndp Sep 3, 2018 •

edited

Loading

dmytro-ndp Sep 3, 2018 •

edited

Loading

musienko-maxim Sep 4, 2018 •

edited

Loading

dmytro-ndp Sep 4, 2018 •

edited

Loading

dmytro-ndp commented Sep 4, 2018 •

edited

Loading