-
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
Rework ConvertToProjectWithPomFileTest test for checking conversion folders to maven multimodule project #11427
Conversation
*/ | ||
package org.eclipse.che.selenium.miscellaneous; | ||
|
||
import static java.lang.String.*; |
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.
As far as I know, the common import is a bad practice. Please import relations directly.
import org.testng.annotations.BeforeClass; | ||
import org.testng.annotations.Test; | ||
|
||
/** @author Aleksandr Shmaraev */ |
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.
Is the author name correct?
convertPredefinedFolderToMavenProjectWithContextMenu( | ||
format("%s/%s", PROJECT_NAME, WEB_APP_MODULE)); | ||
testProjectServiceClient.checkProjectType( | ||
workspaceId, format("%s/%s", PROJECT_NAME, WEB_APP_MODULE), "maven"); |
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.
What about move all these items to the upper of the test and declare them as constants.
final String convertPath = format("%s/%s", PROJECT_NAME, WEB_APP_MODULE);
workspaceId, format("%s/%s", PROJECT_NAME, WEB_APP_MODULE), "maven"); | ||
addParentConfigurationToPredefinedFolder(); | ||
menu.runCommand( | ||
TestMenuCommandsConstants.Project.PROJECT, |
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.
Static import?
public void shouldNotConvertToMavenProject() { | ||
projectExplorer.waitAndSelectItem(NONE_MAVEN_PROJECT); | ||
menu.runCommand( | ||
TestMenuCommandsConstants.Project.PROJECT, |
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.
Static import?
@@ -293,6 +293,7 @@ private ProjectConfig getProject(String workspaceId, String projectName) throws | |||
return requestFactory | |||
.fromUrl(workspaceAgentApiEndpointUrlProvider.get(workspaceId) + "project/" + projectName) | |||
.useGetMethod() | |||
.setAuthorizationHeader(machineServiceClient.getMachineApiToken(workspaceId)) |
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.
Should there be added prefix BEARER_TOKEN_PREFIX +
as well?
@@ -25,6 +25,7 @@ | |||
public static final String CONSOLE_JAVA_SIMPLE = "console_java_simple.json"; | |||
public static final String GO = "go.json"; | |||
public static final String DOT_NET = "dotNet.json"; | |||
public static final String UNDEFINED_PROJECT = "undefined.json"; |
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.
IMHO next name would better reflect template proposal: PROJECT_OF_UNDEFINED_TYPE
@Inject private TestProjectServiceClient testProjectServiceClient; | ||
@Inject private SeleniumWebDriver seleniumWebDriver; | ||
private String workspaceId; | ||
@BeforeClass |
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, add empty line to improve readability.
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.
Done
convertToMavenByhWizard("/" + PROJECT_NAME, WEB_APP_MODULE); | ||
} | ||
|
||
private void convertToMavenByhWizard(String pathToDirectory, String convertedFolder) { |
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 convertToMavenByhWizard
Also, this method could be useful for other tests, and would be better moved into the Wizard page object.
private static final String PROJECT_NAME = NameGenerator.generate("project", 4); | ||
private static final String WEB_APP_MODULE = "my-webapp"; | ||
private static final String NONE_MAVEN_PROJECT = NameGenerator.generate("noneMavenProject", 4); | ||
private static final String PARENT_INFORMATION = |
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.
I would suggest to use more specific name PARENT_ARTIFACT_SECTION
.
wizard.waitCloseProjectConfigForm(); | ||
} | ||
|
||
private void addParentConfigurationToPredefinedFolder() { |
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.
I would suggest to use more specific name addParentArticatSectionIntoPomFile
import org.testng.annotations.Test; | ||
|
||
/** @author Aleksandr Shmaraev, Musienko Maxim */ | ||
public class CheckConvertingToMavenProjectTest { |
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.
Test name could be simplified slightly: ConvertToMavenProjectTest
ci-build |
ci-build |
…olders to maven multimodule project (eclipse-che#11427) Rework ConvertToProjectWithPomFileTest test for checking conversion folders to maven multimodule project
What does this PR do?
artifactId
was the same for parent and module and this produces conflict)undefined
project type for checking autosetting type for projects like thisWhat issues does this PR fix or reference?
#6905