-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement selenium test for covering LS server for PHP language #9805
Conversation
ci-build |
editor.waitActive(); | ||
} | ||
|
||
@Test() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
() looks redundant here
} | ||
|
||
private void checkAutocompletion() { | ||
editor.goToCursorPositionVisible(13, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, divide separate steps by empty rows to improve readability.
} | ||
|
||
private void checkCodeValidation() { | ||
editor.goToPosition(14, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, divide separate steps by empty rows to improve readability.
import org.testng.annotations.Test; | ||
|
||
/** @author Musienko Maxim */ | ||
public class PhpLanguageServerTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually have test for another language with completely different name CheckMainFeatureForCSharpLanguageTest
.
Let's unify naming scheme. In fact, we don't test language server itself. So server in test name looks confusing, as for me.
What about next test names?
- CSharpFileEditingTest
- PhpFileEditingTest
String.format( | ||
"Finished language servers initialization, file path '/%s'", PATH_TO_INDEX_PHP); | ||
consoles.waitExpectedTextIntoConsole(intitPhpLanguageServerMessage); | ||
checkCodeValidation(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, divide separate steps by empty rows to improve readability.
|
||
@Test() | ||
public void checkMainFeaturesPhpLS() { | ||
String intitPhpLanguageServerMessage = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo here: "intit"
|
||
@BeforeClass | ||
public void setup() throws Exception { | ||
URL resource = getClass().getResource("/projects/plugins/DebuggerPlugin/php-tests"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can define this URL at the class loading time. So, it's better to extract this URL as a constant.
…mendation for the PR
@Test() | ||
public void checkMainFeaturesPhpLS() { | ||
@Test | ||
public void PhpFileEditingTest() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
old name of test looked better
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4538/ |
ci-build |
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4539/ |
…pse-che#9805) * Implement selenium test for covering LS server for PHP
What does this PR do?
Find definition
command. Have been changedwebDriverWaitFactory
toseleniumWebDriverHelper
for better readability and simplify codeWhat issues does this PR fix or reference?
#9760