-
Notifications
You must be signed in to change notification settings - Fork 0
refactoring Sv_Selenium #6
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
base: master
Are you sure you want to change the base?
Conversation
|
@SvNov fix spaces and tabs/whitespace characters - no more that one empty line or one tabs/whitespace between elements |
|
@SvNov Don't name variables as |
| { | ||
| class CreateSavedSearchPageTest : BaseTest | ||
| { | ||
| private LoginPage objLoginPage; |
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 method can be simplified if you use parameterized selector:
create method that returns selector for list view item by its name:
private By ListViewItem(string itemName) => By.Xpath($"//span[contains(@class,'lvName-span')][@title='{itemName}']");
then here:
WebDriverWait wait = new WebDriverWait(driver, TimeSpan.FromSeconds(15));
wait.Until(x => driver.FindElement(ListViewItem(criteria));
)
you can either make this method void - it will throw exception if item is not found - or wrap it into try catch and return true/false (and make assert statement in test)
| objSearchResultsPage = new SearchResultsPage(driver, waiter); | ||
|
|
||
| // performs simple search | ||
| objSearchResultsPage.Search("4819-5337-4080"); |
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 data (strings, ids names etc) needs to be moved out into test class fields with meaningful name. Therefore we can quickly see what hard coded test data test depends on.
| objCreateSavedSearchPage = new CreateSavedSearchPage(driver, waiter); | ||
| objCreateSavedSearchPage.ClickOnContainerOptionsButton(); | ||
|
|
||
| //var docName = objSearchResultsPage.GetSearchResultDocumentName("4819-5337-4080"); |
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.
remove all commented code
| private readonly By locator = By.Id("containerOptionsButton"); | ||
| private readonly By okButton = By.Name("okButton"); | ||
|
|
||
| public IWebElement ContainerOptionsButton => |
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 either define Selectors (By) and then find elements in your methods. Or you define web elements (IWebElement) adn use those in your methods but not both - it's unnecessary complication.
| { | ||
| var search = waiter.Until(SeleniumExtras.WaitHelpers | ||
| .ExpectedConditions | ||
| .ElementToBeClickable(By.Id("nd-hsCriteria-input"))); |
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.
hardcoded selector
| { | ||
| var documentRow = waiter.Until(SeleniumExtras.WaitHelpers | ||
| .ExpectedConditions | ||
| .ElementIsVisible(By.ClassName("id_"+ searchText))); |
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.
hardcoded selector - change it to parameterized selector:
private By ListViewItem(string itemName) => By.Xpath($"//span[contains(@class,'lvName-span')][@title='{itemName}']");
then here:
WebDriverWait wait = new WebDriverWait(driver, TimeSpan.FromSeconds(15));
wait.Until(x => driver.FindElement(ListViewItem(criteria));
)
| executeSearch.Click(); | ||
| } | ||
|
|
||
| public string GetSearchResultDocumentName(string searchText) |
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 signature doesn't make sense - you are getting document name by giving this method document name?
change it public bool IsListViewItemDisplayed(string documentName)
|
|
||
| } | ||
|
|
||
| public void ClickOnContainerOptionsButton() |
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.
change this method to accept string with option name that you need to click
|
|
||
| objCreateSavedSearchPage.ClickSaveSearchSubMenu(); | ||
| objCreateSavedSearchPage.ClickOnOkButton(); | ||
| objCreateSavedSearchPage.WaitSaveSearchPage(); |
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 has to be Assert.IsTrue(UI.SearchResultsPage.IsListViewItemDisplayed(savedSearchName);
|
@SvNov Create separate Create |
|
another change to add (from our Skype conversation):
|
No description provided.