Skip to content
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

NameSpace Admin page locators actions and step definition addition. #225

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

rahuldash171
Copy link
Contributor

Namespace admin page FW changes for new locators , actions and step defs .

* keyValuePair as key
*/
public static void enterKeyValuePreferences(String preferenceProperty, String keyValuePair) {
String pluginPropertyDataCyAttribute = PluginPropertyUtils.getPluginPropertyDataCyAttribute(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is plugin being referenced on namespace admin page?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ankit , This plugin is for setting the nameSpace admin preferences which are key-value pairs present inside datacy properties file .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the naming to String dataCyAttribute .

Comment on lines 55 to 65
public static WebElement locateKeyProperty(String pluginProperty, int row) {
String xpath = "//*[@data-cy='" + pluginProperty + "" + row + "']//input[@placeholder='key']";
return SeleniumDriver.getDriver().findElement(By.xpath(xpath));
}

public static WebElement locateValueProperty(String pluginProperty, int row) {
String xpath = "//*[@data-cy='" + pluginProperty + "" + row + "']//input[@placeholder='value']";
return SeleniumDriver.getDriver().findElement(By.xpath(xpath));
}

public static WebElement locateAddRowButtonProperty(String pluginProperty, int row) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment applies to this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok ankit , I have done the necessary changes . Please have a look in the latest commit . Thanks .

* {@link ConstantsUtil#DEFAULT_PLUGIN_PROPERTIES_FILE} with
* keyValuePair as key
*/
public static void enterKeyValuePreferences(String preferenceProperty, String keyValuePair) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same method already exists here:

public static void enterKeyValuePairsForProperty(String pluginProperty, String jsonKeyValuePairs) {

Please avoid code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-08-31 at 19 10 56 Screenshot 2023-08-31 at 19 13 10

Ankit , here the locators are different . The method which is present inside cdfPluginPropertiesActions is pointing to setting runtime arguments for a deployed pipline whereas my method is pointing to setting preferences inside namespace admin . The key and value tabs which are present here does not work with the old locators .

/**
* Represents CdfNameSpaceAdminLocators
*/
public class CdfNameSpaceAdminLocators {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please get a review on locators class from @GnsP from UI team.

Comment on lines +85 to +103
String dataCyAttribute = PluginPropertyUtils.getPluginPropertyDataCyAttribute(
preferenceProperty);
if (dataCyAttribute == null) {
dataCyAttribute = preferenceProperty;
}
Map<String, String> properties =
JsonUtils.convertKeyValueJsonArrayToMap(PluginPropertyUtils.pluginProp(keyValuePair));
int index = 0;
for (Map.Entry<String, String> entry : properties.entrySet()) {
if (index != 0) {
ElementHelper.clickOnElement(CdfNameSpaceAdminLocators.locateAddRowButtonProperty(
dataCyAttribute, index - 1));
}
ElementHelper.sendKeys(CdfNameSpaceAdminLocators.locateKeyProperty(
dataCyAttribute, index), entry.getKey());
ElementHelper.sendKeys(CdfNameSpaceAdminLocators.locateValueProperty(
dataCyAttribute, index), entry.getValue());
index++;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-cy attributes are specific to tests written using cypress. Prefer using using a custom data- attribute for this action.

expectedNameSpace);
}

public static void verifyElementIsDisplayed() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method name seems to indicate that this method checks if any given element is displayed, where as it actually checks if the page header is displayed or not. Consider changing the method name to reflect its actual behavior.

*/
public class CdfNameSpaceAdminLocators {

@FindBy(how = How.XPATH, using = "//div[contains(text(),'Namespace Admin')]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding elements using text / copy is generally not a good idea, as copy may change based on product requirements and this may result in breaking tests across multiple projects (as this locator is in the e2e framework). Prefer using a data-testid for locating elements.

@FindBy(how = How.XPATH, using = "//div[contains(text(),'Namespace Admin')]")
public static WebElement namespaceAdminMenu;

@FindBy(how = How.XPATH, using = "//*[@data-cy='navbar-hamburger-icon']")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer data-testid over data-cy.

return SeleniumDriver.getDriver().findElement(By.xpath(path));
}

public static WebElement locateMenuLink(String menuLink) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer data-testid over data-cy

@FindBy(how = How.XPATH, using = "//span[contains(text(),'Add Connection')]")
public static WebElement addConnectionTab;

@FindBy(how = How.XPATH, using = "//span[contains(text(),'Import')]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locating elements based on text content is not a good idea, as explained in other comments.

@FindBy(how = How.XPATH, using = "//span[contains(text(),'Import')]")
public static WebElement importConnectionTab;

@FindBy(how = How.XPATH, using = "//span[contains(text(),'Create Profile')]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locating elements based on text content is not a good idea, as explained in other comments.

@FindBy(how = How.XPATH, using = "//span[contains(text(),'Create Profile')]")
public static WebElement createProfileInNamespaceAdmin;

@FindBy(how = How.XPATH, using = "//a[contains(text(),\"Switch to\")]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locating elements based on text content is not a good idea, as explained in other comments.

@FindBy(how = How.XPATH, using = "//a[contains(text(),\"Switch to\")]")
public static WebElement switchToNameSpaceButton;

@FindBy(how = How.XPATH, using = "//div[@class=\"namespace-and-caret\"]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locating elements based on css classes is not a good idea, as explained in other comments.

@FindBy(how = How.XPATH, using = "//div[@class=\"namespace-and-caret\"]")
public static WebElement namespaceText;

@FindBy(how = How.XPATH, using = "//*[@data-cy='feature-heading'][//div[contains(text(),'Namespace')]]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locating elements based on text content is not a good idea, as explained in other comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants