Skip to content

Implement selenium test for covering LS server for TypeScript language#9969

Merged
musienko-maxim merged 10 commits intomasterfrom
CHE-9918
Jun 10, 2018
Merged

Implement selenium test for covering LS server for TypeScript language#9969
musienko-maxim merged 10 commits intomasterfrom
CHE-9918

Conversation

@musienko-maxim
Copy link
Copy Markdown
Contributor

What does this PR do?

This PR implements simple scenario: create a workspace with enabled TypeScript LS installer, open simple TypeScript project, check initialization TypeScript LS by UI, checks code validation, autocompletion, go to definition features.

What issues does this PR fix or reference?

#9918

@musienko-maxim musienko-maxim requested a review from vparfonov as a code owner June 8, 2018 09:07
@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Jun 8, 2018
}

/**
* Waits marker with specified {@code markerLocator} on the defined {@code position} and move
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.

position could confuse a little so as it's about the line number, not just about specific position in line.

}

private void checkCodeValidation() {
final int expectedValueOfErrorMarkers = 9;
Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 8, 2018

Choose a reason for hiding this comment

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

from the readability point it's better to divide verification by given/when/then sections.

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.

Maybe "expectedCountOfErrorMarkers" better?

}

private void checkCodeAssistant() {
String textFromWholeCodeAssistantScope =
Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 8, 2018

Choose a reason for hiding this comment

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

from the readability point it's better to divide verification by given/when/then sections

Copy link
Copy Markdown
Contributor Author

@musienko-maxim musienko-maxim Jun 8, 2018

Choose a reason for hiding this comment

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

I have not quite understood what do you mean?
use something like that:

 // when
    projectExplorer.openItemByPath(PATH_TO_PROGRAM);
    editor.setInactiveBreakpoint(22);
    editor.closeAllTabs();
    commandsPalette.openCommandPalette();
    commandsPalette.startCommandByDoubleClick(MAKE_AND_DEBUG_COMMAND_NAME);
    consoles.waitExpectedTextIntoConsole("Listening on port 8001");

    menu.runCommandByXpath(
        TestMenuCommandsConstants.Run.RUN_MENU,
        TestMenuCommandsConstants.Run.DEBUG,
        getXpathForDebugConfigurationMenuItem());

    notifications.waitExpectedMessageOnProgressPanelAndClosed(
        String.format("Remote debugger connected\nConnected to: localhost:%s.", DEBUG_PORT));

    // then
    editor.waitTabFileWithSavedStatus("hello.cc");
    debugPanel.waitDebugHighlightedText("  return \"Hello World, \" + name + \"!\";");
    debugPanel.waitTextInVariablesPanel("name =");
    debugPanel.waitTextInVariablesPanel("\"man\"");

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.

yes

editor.waitTextIntoAutocompleteContainer(textFromWholeCodeAssistantScope);
editor.closeAutocomplete();
editor.typeTextIntoEditor(ENTER.toString());
editor.typeTextIntoEditor("greeter.");
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.

it's hard to understand test case here
Could you, please, name string "greeter." somehow to make its propose an obvious?

Copy link
Copy Markdown
Contributor Author

@musienko-maxim musienko-maxim Jun 8, 2018

Choose a reason for hiding this comment

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

In the test based on simple Greeter example like:
Greeter example

class Greeter {
    greeting: string;
    constructor(message: string) {
        this.greeting = message;
    }
    greet() {
        return "Hello, " + this.greeting;
    }
    
     testPrint(): void {
        const printVar = new Print();
        printVar.print("test print");
    }
}

let greeter = new Greeter("world");

and i cannot see difficult parts for understanding

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.

To protect the test from occasional breaking down we need to have clear vision why it cann't be "hello", "buy" and any other string :-)

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.

Ok, your proposal?

import static org.eclipse.che.selenium.core.workspace.WorkspaceTemplate.ECLIPSE_NODEJS;
import static org.eclipse.che.selenium.pageobject.CodenvyEditor.MarkerLocator.ERROR;
import static org.eclipse.che.selenium.pageobject.CodenvyEditor.MarkerLocator.ERROR_OVERVIEW;
import static org.openqa.selenium.Keys.*;
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.

This is a kind of anti-pattern, please use direct static import

"The expected value of errors marker should be %d but actual %d",
expectedValueOfErrorMarkers, actualValueErrorMarkers));
editor.waitMarkerInPositionAndMoveCursor(ERROR_OVERVIEW, 13);
editor.waitTextInToolTipPopup("Cannot find name 'c'");
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.

it's hard to understand test case here
Could you, please, name string "Cannot find name 'c'" somehow to make its propose an obvious?

Copy link
Copy Markdown
Contributor Author

@musienko-maxim musienko-maxim Jun 8, 2018

Choose a reason for hiding this comment

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

This message is returned by Language Server and we can not control this one in this case

Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 8, 2018

Choose a reason for hiding this comment

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

I didn't mean control - I meant name this string somehow to make its propose clear, and we can do it in our test.

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.

OK, agree

}

private void checkGoToDefinition() {
editor.goToPosition(24, 20);
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.

Could you, please, comment in the code which definition we a going to here?

* @param markerLocator marker's type, defined in {@link MarkerLocator}
* @param position line's number, where marker is expected
*/
public void waitMarkerInPositionAndMoveCursor(MarkerLocator markerLocator, int position) {
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.

moveCursorToLineWithMarker(MarkerLocator markerLocator, int line) would explain the propose of method better, IMHO

}

private void checkCodeAssistant() {

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.

redundant empty line


String nameOfGreeterClassRef = "greeter.";

editor.goToPosition(28, 36);
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.

could you, please divide separate steps by empty lines to improve readability?

@musienko-maxim
Copy link
Copy Markdown
Contributor Author

ci-build

@codenvy-ci
Copy link
Copy Markdown

@musienko-maxim
Copy link
Copy Markdown
Contributor Author

ci-test

@codenvy-ci
Copy link
Copy Markdown

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9969
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@musienko-maxim musienko-maxim merged commit 60f123d into master Jun 10, 2018
@musienko-maxim musienko-maxim deleted the CHE-9918 branch June 10, 2018 20:53
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jun 10, 2018
@benoitf benoitf added this to the 6.7.0 milestone Jun 10, 2018
hbhargav pushed a commit to hbhargav/che that referenced this pull request Dec 5, 2018
eclipse-che#9969)

* finish the test and adapt files from resources for the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/task Internal things, technical debt, and to-do tasks to be performed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants