-
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
Selenium: Create one test from "ImportRecursiveSubmoduleTest", "ImportProjectIntoSpecifiedBranchTest", and "KeepDirectoryGitImportTest" selenium tests #9372
Conversation
@@ -239,4 +307,81 @@ private void createFile(Path pathToRootContentDirectory, Path pathToFile) throws | |||
public String getFileContent(String pathToFile) throws IOException { | |||
return IOUtils.toString(ghRepo.getFileContent(pathToFile).read(), "UTF-8"); | |||
} | |||
|
|||
public String getRepoSha() throws IOException { |
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 rather name it getDefaultBranchSha()
.
Alsod Repo
term is redundant at all so as it is the method of TestGitHubRepository
class.
return String.format(modulePattern, repoName, pathToSubmodule, repoUrl); | ||
} | ||
|
||
private void createGitModulesFile(TestGitHubRepository targetRepository, String pathForSubmodule) |
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 link to the doc with explanation what .gitmodules file is about. I would also recommend to use more general name of method like "setupSubmoduleConfig()".
Also targetRepository
parameter name is a little bit misleading. It is rather submodule
, IMHO.
Also pathToSubmodule
is rather pathToSubmoduleContent
.getSha(); | ||
} | ||
|
||
private String getSubmoduleConfig(TestGitHubRepository targetRepository, String pathToSubmodule) { |
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.
targetRepository
parameter name is a little bit misleading. It is rather submodule, IMHO.
Also pathToSubmodule
is rather pathToSubmoduleContent
currentProjectName = importBranchRepo.getName(); | ||
projectExplorer.waitProjectExplorer(); | ||
|
||
importIntoBranchFromGitHub(); |
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.
It's hard to understand test scenario when we have just list of 27 commands.
Please divide separate steps with empty rows and comments.
menu.runCommand( | ||
TestMenuCommandsConstants.Workspace.WORKSPACE, | ||
TestMenuCommandsConstants.Workspace.IMPORT_PROJECT); | ||
importProject.waitMainForm(); |
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.
It's hard to understand test scenario when we have just list of tens of commands.
Please divide separate steps with empty rows and comments.
importProject.closeWithIcon(); | ||
events.clickEventLogBtn(); | ||
events.waitExpectedMessage("Failed to authorize application on GitHub"); | ||
fail("Known issue https://github.com/eclipse/che/issues/8288", ex); |
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.
It seems outdated verification because #8288 had been closed.
projectExplorer.waitItem(projectName); | ||
} | ||
|
||
private void openSubmoduleOne(String projectName) throws Exception { |
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.
It's better not to enclose submodule number into the method name because it makes it hard to reorder the modules in future.
editor.closeFileByNameWithSaving("GreetingController"); | ||
} | ||
|
||
private void openSubmoduleTwo(String projectName) throws Exception { |
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.
It's better not to enclose submodule number into the method name because it makes it hard to reorder the modules in future.
public class ImportWizardFormTest { | ||
private static final Logger LOG = LoggerFactory.getLogger(ImportWizardFormTest.class); | ||
private static final String GITHUB_COM = "github.com"; | ||
private static final String FIRST_BRANCH = "firstBranch"; |
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.
It's better not to enclose number into the constant name because it makes it hard to reorder the branches in the future.
private static final String FIRST_BRANCH = "firstBranch"; | ||
private static final String SECOND_BRANCH = "secondBranch"; | ||
private static final String THIRD_BRANCH = "thirdBranch"; | ||
private static final String FIRST_SUBMODULE_NAME = "Repo_For_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.
It's better not to enclose number into the constant name because it makes it hard to reorder the modules in future.
@@ -58,12 +58,9 @@ | |||
<class name="org.eclipse.che.selenium.dashboard.organization.AddWorkspaceToOrganizationTest"/> | |||
<class name="org.eclipse.che.selenium.dashboard.organization.ShareWorkspaceTest"/> | |||
<class name="org.eclipse.che.selenium.git.AuthorizeOnGithubFromDashboardTest"/> | |||
<class name="org.eclipse.che.selenium.git.AuthorizeOnGithubFromImportTest"/> | |||
<class name="org.eclipse.che.selenium.git.ImportWizardFormTest"/> |
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.
ImportWizardFormTest
can be moved into the CheSuite.xml so as it's expected to be ready for multi-thread execution.
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.
I think, need to add @author - Igor Ohrimenko
|
||
menu.runCommand( | ||
TestMenuCommandsConstants.Workspace.WORKSPACE, | ||
TestMenuCommandsConstants.Workspace.IMPORT_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.
I think, it would be nice to use static import and replace in menu.runCommand -
e.g.
menu.runCommand(WORKSPACE, IMPORT_PROJECT);
The code will be more small and readable in this case
ci-test |
ci-test build report: |
ci-test |
ci-test build report: |
What does this PR do?
Create one test from "ImportRecursiveSubmoduleTest", "ImportProjectIntoSpecifiedBranchTest", and "KeepDirectoryGitImportTest" selenium tests. And rework them with "kohsuke" library, for moving to the "multi-thread" suite.
What issues does this PR fix or reference?
Issues: #9150 ; #9149 ; #9151
Release Notes
Docs PR