-
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: Rework the 'CheckoutReferenceTest' to run in the multi-thread mode #9368
Conversation
ci-test |
String hashCommit = sha1.substring(0, 8); | ||
String wrongHashCommit = String.format("%s ##", hashCommit); | ||
String failMessage = String.format("Branch name %s is not allowed", wrongHashCommit); | ||
// import the test repo |
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.
missed empty line before comment
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.
You are right
* | ||
* @param refName is a name of branch, tag, etc | ||
* @param branchName is a name of new branch |
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.
@param branchName name of the branch which should be created
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.
OK
/** Returns all cells from top row divided by "\n" */ | ||
private String getGitHistoryTopContent() { | ||
String rowTopIndex = Integer.toString(0); | ||
return seleniumWebDriverHelper |
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.
There is a method SeleniumWebDriverHelper#waitVisibilityAndGetText()
which is more suitable here.
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.
OK
GHRef master = ghRepo.getRef("heads/master"); | ||
return ghRepo.createRef("refs/heads/" + refName, master.getObject().getSha()); | ||
public GHRef createBranch(String branchName) throws IOException { | ||
GHRef defaultBranch = ghRepo.getRef("heads/" + ghRepo.getDefaultBranch()); |
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 create public method which will be return the default branch
GHRef public getDefaultBranch(){
return ghRepo.getRef("heads/" + ghRepo.getDefaultBranch());
}
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.
Which test needs it?
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.
If there are actually no tests which need getDefaultBranch()
, I would not create such method because we have already had ghRepo.getDefaultBranch()
.
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.
He can create private method. Multiple using the same row
GHRef defaultBranch = ghRepo.getRef("heads/" + ghRepo.getDefaultBranch());
looks not very good
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.
Method name getReferenceToDefaultBranch()
is more relevant to the returning value, IMHO
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 agree
ci-test build report: |
What does this PR do?
What issues does this PR fix or reference?
#9139, #8153