Add selenium tests for covering advanced operations for C# LS#11261
Add selenium tests for covering advanced operations for C# LS#11261musienko-maxim merged 16 commits intomasterfrom
Conversation
| private static final String PROJECT_NAME = | ||
| NameGenerator.generate(CSharpClassRenamingTest.class.getSimpleName(), 4); | ||
|
|
||
| private static final String PATH_TO_DOTNET_FILE = PROJECT_NAME + "/Hello.cs"; |
There was a problem hiding this comment.
What about "PATH_TO_DOTNET_FILE" >>> "PATH_TO_DOT_NET_FILE"
| String expectedTextInHoverPopUp = | ||
| "System.Console\nRepresents the standard input, output, and error streams for console applications. This class cannot be inherited."; | ||
| editor.moveCursorToText("Console"); | ||
| Assert.assertEquals(editor.getTextFromHoverPopup(), expectedTextInHoverPopUp, "https://github.com/eclipse/che/issues/10117"); |
There was a problem hiding this comment.
Better change to "editor.waitTextInHoverPopup(expectedTextInHoverPopUp);" this increase stability of this test
There was a problem hiding this comment.
waitTextInHoverPopup does not work in this case because use this: seleniumWebDriverHelper.waitTextContains(hoverPopup, expectedText);. In this case check full text matching because text is duplicated.
Moreover editor.getTextFromHoverPopup() already had visibility waiting and pretty stable as for me
There was a problem hiding this comment.
In that case we could make both editor.waitTextInHoverPopupEqualsTo(expectedText) and editor.waitTextInHoverPopupContains(expectedText)
There was a problem hiding this comment.
Ok, I understand, but you definitely will have a problem with this assert because it tries to get text exactly once and if something doesn't display, test will be failed.
What about adding a new method to page object with "SeleniumWebDriverHelper#waitTextEqualsTo"
| public void checkFindDefinition() { | ||
| editor.goToCursorPositionVisible(21, 18); | ||
| menu.runCommand(ASSISTANT, FIND_DEFINITION); | ||
| editor.waitTabFileWithSavedStatus("Test.cs"); |
There was a problem hiding this comment.
Did you mean "editor.waitTabIsPresent("Test.cs");"?
|
|
||
| @Test(priority = 3, alwaysRun = true) | ||
| public void checkGoToSymbolFeature() { | ||
| menu.runCommand(ASSISTANT, FIND_DEFINITION); |
There was a problem hiding this comment.
menu.runCommand(ASSISTANT, FIND_DEFINITION); -> menu.runCommand(ASSISTANT, GO_TO_SYMBOL);
...st/java/org/eclipse/che/selenium/languageserver/csharp/CSharpFileAdvancedOperationsTest.java
Show resolved
Hide resolved
| @Test(priority = 2, alwaysRun = true) | ||
| public void checkCodeCommentFeature() { | ||
| editor.goToPosition(17, 1); | ||
| editor.typeTextIntoEditor(CONTROL.toString() + "/"); |
There was a problem hiding this comment.
As I know it doesn't work. Please change to:
String comment = Keys.chord(CONTROL, "/");
seleniumWebDriverHelper.sendKeys(comment);
There was a problem hiding this comment.
Doesn't work where? More details please.
There was a problem hiding this comment.
Did you test this? If it works then OK.
There was a problem hiding this comment.
Yes it works in my case in grid and local mode
There was a problem hiding this comment.
I would recommend to use existed method CodenvyEditor#launchCommentCodeFeature() to comment the line.
| String expectedTextInHoverPopUp = | ||
| "System.Console\nRepresents the standard input, output, and error streams for console applications. This class cannot be inherited."; | ||
| editor.moveCursorToText("Console"); | ||
| Assert.assertEquals(editor.getTextFromHoverPopup(), expectedTextInHoverPopUp, "https://github.com/eclipse/che/issues/10117"); |
There was a problem hiding this comment.
Also I would make error message clearer, like the follow:
Known random failure https://github.com/eclipse/che/issues/10117
|
|
||
| @Test(priority = 1, alwaysRun = true) | ||
| public void checkFindDefinition() { | ||
| editor.goToCursorPositionVisible(21, 18); |
There was a problem hiding this comment.
Could you, please, comment, which which thing we are trying to find definition of here?
| return seleniumWebDriverHelper.waitVisibilityAndGetText(hoverPopup); | ||
| } | ||
|
|
||
| /** Get text from hover popup */ |
There was a problem hiding this comment.
/** Get text from hover popup / --> /* Wait text from hover popup*/
716647b to
d5e3b40
Compare
…To(String expectedText)' method
|
|
||
| @BeforeClass | ||
| public void setUp() throws Exception { | ||
| URL resource = getClass().getResource("/projects/CSharpFileAdvancedOperations"); |
There was a problem hiding this comment.
Could you, please, comment on what we are trying to achieve here?
There was a problem hiding this comment.
In my opinion, just renaming of variable to something like "testProject" will be enough.
There was a problem hiding this comment.
I meant setUp() method itself - what it is actually setting up?
There was a problem hiding this comment.
For example the same: https://github.com/eclipse/che/blob/master/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/debugger/PhpProjectDebuggingTest.java#L75
What are we setting up here?
There was a problem hiding this comment.
Well, I am talking about state of IDE which we are achieving after the method is executed.
According to the code, I would write that the method "is opening PATH_TO_DOTNET_FILE, then waits initialization of Language Server, and then waits until "/obj" and "/bin" directories are visible in ProjectExplorer".
There was a problem hiding this comment.
Ah, i have understood now. So i've added comment, check it please
| String expectedTextInHoverPopUp = | ||
| "System.Console\nRepresents the standard input, output, and error streams for console applications. This class cannot be inherited."; | ||
| editor.moveCursorToText("Console"); | ||
| Assert.assertEquals(editor.getTextFromHoverPopup(), expectedTextInHoverPopUp, "https://github.com/eclipse/che/issues/10117"); |
There was a problem hiding this comment.
In that case we could make both editor.waitTextInHoverPopupEqualsTo(expectedText) and editor.waitTextInHoverPopupContains(expectedText)
| seleniumWebDriverHelper.waitTextEqualsTo(hoverPopup, expectedText); | ||
| } catch (TimeoutException ex) { | ||
| // remove try-catch block after issue has been resolved | ||
| fail("Known issue https://github.com/eclipse/che/issues/10117", ex); |
There was a problem hiding this comment.
Do you happen to know if it is permanent or random failure?
ab54127 to
279453b
Compare
|
ci-build |
|
ci-build |
What does this PR do?
What issues does this PR fix or reference?
#11202