-
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: cover sharing workspaces in organization by selenium test #9167
Conversation
…kspacesInOrganization
@@ -19,16 +20,16 @@ | |||
class="che-list-item-name"> | |||
<span class="che-xs-header noselect" hide-gt-xs>Email</span> | |||
<span><img class="user-face" gravatar-src="userItemController.user.email"></span> | |||
<span class="user-email che-hover ">{{userItemController.user.email}}</span> | |||
<span class="user-email che-hover " name="member-name">{{userItemController.user.email}}</span> |
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.
Could you rename "member-name" to "member-email"
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.
Also I would notice that it is about user, not member.
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.
name
attribute is usually used in input elements. It's better to use id
for div element.
@@ -1,5 +1,6 @@ | |||
<che-list-item flex-gt-sm="100" flex="33" ng-mouseover="hover=true" ng-mouseout="hover=false"> | |||
<div flex="100" | |||
id="member-name-{{userItemController.user.email}}" |
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 about user email, not member name.
</div> | ||
<div flex-gt-xs="35"> | ||
<span class="che-xs-header noselect" hide-gt-xs>Permissions</span> | ||
<span class="user-permissions">{{userItemController.getUserActions()}}</span> | ||
<span class="user-permissions" name="member-permissions">{{userItemController.getUserActions()}}</span> |
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 is about user, not member.
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.
name
attribute is usually used in input elements. It's better to use id
for div element.
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.
Each member has permissions and id is not suitable here.
</div> | ||
<div flex-gt-xs="25"> | ||
<span class="che-xs-header noselect" hide-gt-xs>Actions</span> | ||
<span class="che-list-actions"> | ||
<div ng-click="userItemController.removeUser();" uib-tooltip="Remove member"> | ||
<div ng-click="userItemController.removeUser();" uib-tooltip="Remove member" name="remove-member"> |
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.
name
attribute is usually used in input elements. It's better to use id
for div element.
import org.testng.annotations.BeforeMethod; | ||
import org.testng.annotations.Test; | ||
|
||
@Test(groups = {TestGroup.MULTIUSER}) |
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 can omit {}
brackets here for single group: Test(groups = TestGroup.MULTIUSER)
import org.openqa.selenium.support.PageFactory; | ||
import org.openqa.selenium.support.ui.WebDriverWait; | ||
|
||
public class WorkspaceShare { | ||
private final SeleniumWebDriver seleniumWebDriver; |
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 don't need separate field seleniumWebDriver
while we use seleniumWebDriverHelper
only.
|
||
@Inject | ||
public WorkspaceShare(SeleniumWebDriver seleniumWebDriver) { | ||
public WorkspaceShare( | ||
SeleniumWebDriver seleniumWebDriver, SeleniumWebDriverHelper seleniumWebDriverHelper) { | ||
this.seleniumWebDriver = seleniumWebDriver; |
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.
this.seleniumWebDriver
bacame redundant here
seleniumWebDriverHelper.waitAndClick(bulkSelection); | ||
} | ||
|
||
public void typeToSearchInput(String value) { |
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 like it is filterMembers
method
} | ||
|
||
@FindBy(xpath = Locators.FILTER_MEMBERS_BY_NAME_FIELD_XPATH) | ||
WebElement filterMembers; |
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.
memberFilter
?
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.
or filterMembersField
?
new WebDriverWait(seleniumWebDriver, REDRAW_UI_ELEMENTS_TIMEOUT_SEC) | ||
.until( | ||
invisibilityOfElementLocated(By.xpath(format(Locators.DEVELOPER_SHARE_ITEM, email)))); | ||
String ADD_DEVELOPER_BTN_XPATH = "//che-button-primary[@che-button-title='Add Developer']"; |
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 don't need to shorten BUTTON
to BTN
, because it doesn't effectively shorten the code, but make it harder to understand.
String ADD_DEVELOPER_BTN_XPATH = "//che-button-primary[@che-button-title='Add Developer']"; | ||
String BULK_SELECTION_ID = "share-workspace-bulk-selection"; | ||
String FILTER_MEMBERS_BY_NAME_FIELD_XPATH = "//input[@ng-placeholder='Search']"; | ||
String MEMBER_ITEM_XPATH = "//div[@id='member-name-%s']"; |
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.
MEMBER_ITEM_XPATH
is actually MEMBER_ITEM_XPATH_PATTERN
which should be completed before usage.
@FindBy(id = Locators.BULK_SELECTION_ID) | ||
WebElement bulkSelection; | ||
|
||
public void clickOnBulk() { |
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.
clickOnBulkSelection
?
assertEquals(workspaceShare.getMemberPermissions(systemAdminName), ADMIN_PERMISSIONS); | ||
|
||
// check selecting member by checkbox | ||
workspaceShare.clickOnMemberCheckbox(systemAdminName); |
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 like a separate test checkSelectingMemberByCheckbox
, IMHO
I would prefer specialized tests to generalized ones.
assertFalse(workspaceShare.isMemberCheckedInList(systemAdminName)); | ||
|
||
// check selecting members by Bulk | ||
workspaceShare.clickOnBulk(); |
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 like a separate test checkMembersBulkSelection
, IMHO
String BULK_SELECTION_ID = "share-workspace-bulk-selection"; | ||
String FILTER_MEMBERS_BY_NAME_FIELD_XPATH = "//input[@ng-placeholder='Search']"; | ||
String MEMBER_ITEM_XPATH = "//div[@id='member-name-%s']"; | ||
String MEMBER_NAME_XPATH = MEMBER_ITEM_XPATH + "//span[@name='member-name']"; |
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 is also looks like xpath pattern
ci-test |
ci-test build report: |
ci-test |
…kspacesInOrganization
ci-test build report: |
What does this PR do?
This PR create ShareWorkspaceTest selenium test which cover sharing workspaces with members of an organization.
What issues does this PR fix or reference?
#9041