[plugin-web-app] Align syntax of element steps #4219
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vkepin
commented
Aug 11, 2023
| */ | ||
| @Deprecated(since = "0.6.0", forRemoval = true) | ||
| @Replacement(versionToRemoveStep = "0.8.0", |
b1638b9 to
b54f655
Compare
Codecov Report
@@ Coverage Diff @@
## master #4219 +/- ##
=========================================
Coverage 97.21% 97.21%
- Complexity 6362 6376 +14
=========================================
Files 889 889
Lines 18317 18356 +39
Branches 1210 1214 +4
=========================================
+ Hits 17807 17845 +38
Misses 400 400
- Partials 110 111 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
valfirst
requested changes
Aug 11, 2023
| * relative paths</i></a> | ||
| * @throws IOException If an input or output exception occurred | ||
| */ | ||
| @When("I select element located by `$locator` and upload file `$filePath`") |
There was a problem hiding this comment.
Suggested change
| @When("I select element located by `$locator` and upload file `$filePath`") | |
| @When("I select element located by `$locator` and upload `$resourceNameOrFilePath`") |
| webDriverProvider.getUnwrapped(RemoteWebDriver.class).setFileDetector(new LocalFileDetector()); | ||
| } | ||
| locator.getSearchParameters().setVisibility(Visibility.ALL); | ||
| baseValidations.assertElementExists(AN_ELEMENT, locator) |
There was a problem hiding this comment.
Suggested change
| baseValidations.assertElementExists(AN_ELEMENT, locator) | |
| baseValidations.assertElementExists("A file input element", locator) |
| } | ||
| locator.getSearchParameters().setVisibility(Visibility.ALL); | ||
| baseValidations.assertElementExists(AN_ELEMENT, locator) | ||
| .ifPresent(browse -> browse.sendKeys(fullFilePath)); |
There was a problem hiding this comment.
Suggested change
| .ifPresent(browse -> browse.sendKeys(fullFilePath)); | |
| .ifPresent(input -> input.sendKeys(fullFilePath)); |
| public void hoverMouseOverElementByLocator(Locator locator) | ||
| { | ||
| baseValidations.assertElementExists( | ||
| String.format(AN_ELEMENT_WITH_ATTRIBUTES, locator), locator) |
There was a problem hiding this comment.
Suggested change
| String.format(AN_ELEMENT_WITH_ATTRIBUTES, locator), locator) | |
| String.format("An element to hover mouse over", locator), locator) |
| @@ -197,7 +306,7 @@ public void doesEachElementByLocatorHaveChildWithLocator(Locator elementLocator, | |||
| * @param cssName A name of the <b>CSS property</b> | |||
| * @param cssValue An expected value of <b>CSS property</b> | |||
| */ | |||
| @Then("the context element has the CSS property '$cssName'='$cssValue'") | |||
| @Then("context element has CSS property `$cssName`=`$cssValue`") | |||
There was a problem hiding this comment.
Suggested change
| @Then("context element has CSS property `$cssName`=`$cssValue`") | |
| @Then("context element has CSS property `$cssName` with value that $comparisonRule `$cssValue`") |
where $comparisonRule is StringComparisonRule
| * @param cssName A name of the <b>CSS property</b> | ||
| * @param cssValue An expected value part of <b>CSS property</b> | ||
| */ | ||
| @Then("context element has CSS property `$cssName` containing `$cssValue`") |
There was a problem hiding this comment.
see the comment above, it would be enough to have only 1 step
| @@ -260,7 +388,7 @@ public boolean doesEachElementByLocatorHaveSameDimension(Locator locator, Dimens | |||
| * has an expected <b>width in a percentage</b> (from style attribute) | |||
| * @param widthInPerc An expected width of the element in a percentage | |||
| */ | |||
| @Then("the context has a width of '$widthInPerc'%") | |||
| @Then("context has width of `$widthInPerc`%") | |||
There was a problem hiding this comment.
Suggested change
| @Then("context has width of `$widthInPerc`%") | |
| @Then("context element has width of $widthPercentage%") |
| * Checks, if the context element has specified width in percentage | ||
| * @param width Expected element with in percentage | ||
| */ | ||
| @Then("context element has width of `$widthInPerc`% relative to parent element") |
There was a problem hiding this comment.
Suggested change
| @Then("context element has width of `$widthInPerc`% relative to parent element") | |
| @Then("context element has width of $widthPercentage% relative to parent element") |
| + " after clicking on the element located by"; | ||
| private static final String ELEMENT_CSS_CONTAINING_VALUE = "Element has CSS property '%s' containing value '%s'"; | ||
| private static final String PARENT_ELEMENT_XPATH = "./.."; | ||
| private static final String PARENT_ELEMENT = "Parent element"; |
There was a problem hiding this comment.
Suggested change
| private static final String PARENT_ELEMENT = "Parent element"; | |
| private static final String PARENT_ELEMENT = "The parent element"; |
f5c4e17 to
c43c2aa
Compare
uarlouski
reviewed
Aug 17, 2023
| } | ||
| File fileForUpload = ResourceUtils.isFileURL(resource.getURL()) ? resource.getFile() | ||
| : unpackFile(resource, filePath); | ||
| if (descriptiveSoftAssert.assertTrue(String.format(FILE_EXISTS_MESSAGE_FORMAT, filePath), |
There was a problem hiding this comment.
we removed file from step naming as per @valfirst comment (imho its weird because each resource is a file and step naming doesn't look good anymore) but inside step we still use "file" in messages
There was a problem hiding this comment.
each resource is a file
nope
step naming doesn't look good anymore
we are adhering common naming: https://docs.vividus.dev/vividus/0.5.13/commons/expressions.html#_calculatefilehash
There was a problem hiding this comment.
- as for the message: it makes sense to update it and make more descriptive
- no need to use
softAssertto check file/resource presence: we can throw exception in case of invalid input - it's considered as user's mistake
valfirst
reviewed
Aug 17, 2023
| */ | ||
| @Deprecated(since = "0.6.0", forRemoval = true) | ||
| @Replacement(versionToRemoveStep = "0.7.0", | ||
| replacementFormatPattern = "Then context element has width of %1$s% relative to parent element") |
There was a problem hiding this comment.
Suggested change
| replacementFormatPattern = "Then context element has width of %1$s% relative to parent element") | |
| replacementFormatPattern = "Then context element has width of %1$s%% relative to parent element") |
uarlouski
reviewed
Aug 21, 2023
| if (!resource.exists()) | ||
| { | ||
| resource = resourceLoader.getResource(ResourceUtils.FILE_URL_PREFIX + filePath); | ||
| } |
There was a problem hiding this comment.
wonder if we can encounter collision when both resource and file exist, the change is nearly 0 but probably we can handle it
There was a problem hiding this comment.
no reason, we always start search in resources
There was a problem hiding this comment.
if needed it can be explicitly described in docs/javadocs
uarlouski
reviewed
Aug 21, 2023
| @When("I click on element located by `$locator` then page does not refresh") | ||
| public void clickElementAndPageNotRefresh(Locator locator) | ||
| { | ||
| baseValidations.assertElementExists(AN_ELEMENT_TO_CLICK, locator).ifPresent(element -> |
There was a problem hiding this comment.
replacing this block with map map would look prettier
uarlouski
reviewed
Aug 21, 2023
| * <li>Clicks on the element;</li> | ||
| * <li>Assert that page has not been refreshed after click</li> | ||
| * </ul> | ||
| * @param locator to locate element |
There was a problem hiding this comment.
@param locator The locator for the upload element
and in other places
66f7d78 to
053df57
Compare
valfirst
requested changes
Aug 29, 2023
| @@ -299,6 +299,294 @@ When I scan barcode from context and save result to scenario variable `qrCodeLin | |||
| Then `${qrCodeLink}` is equal to `https://www.example.com` | |||
| ---- | |||
|
|
|||
| === Element Steps | |||
There was a problem hiding this comment.
Suggested change
| === Element Steps |
Comment on lines
306
to
308
| This step uploads a file with the given relative path. | ||
|
|
||
| NOTE: A relative path starts from some given working directory, avoiding the need to provide the full absolute path (i.e. 'about.jpeg' for the files placed in the src/main/resources directory or '/story/uploadfiles/about.png') |
There was a problem hiding this comment.
Suggested change
| This step uploads a file with the given relative path. | |
| NOTE: A relative path starts from some given working directory, avoiding the need to provide the full absolute path (i.e. 'about.jpeg' for the files placed in the src/main/resources directory or '/story/uploadfiles/about.png') | |
| Uploads the {xref:ROOT:glossary.adoc#_resource}[resource] or file via web interface. |
| When I select element located `$locator` and upload file `$filePath` | ||
| ---- | ||
|
|
||
| * `$locator` - The <<_locator,locator>> to find an element. |
There was a problem hiding this comment.
we need to mention it's required to specify locator for input element with attribute type=file
| .Upload file_for_upload.png file | ||
| [source,gherkin] | ||
| ---- | ||
| When I select element located `By.id(uploadfile)` and upload file `/folder/file_for_upload.png` |
There was a problem hiding this comment.
Suggested change
| When I select element located `By.id(uploadfile)` and upload file `/folder/file_for_upload.png` | |
| When I select element located by `id(uploadfile)` and upload file `/folder/file_for_upload.png` |
|
|
||
| === Verify page is not refreshed after click on element | ||
|
|
||
| This step clicks on the element and checks that the page has not been refreshed after clicking. |
There was a problem hiding this comment.
Suggested change
| This step clicks on the element and checks that the page has not been refreshed after clicking. | |
| Clicks on the element and checks that the page has not been refreshed after the click. |
|
|
||
| === Check an element CSS property | ||
|
|
||
| Check that the context element has an expected CSS property. |
There was a problem hiding this comment.
Suggested change
| Check that the context element has an expected CSS property. | |
| Checks the context element has the expected CSS property. |
|
|
||
| Check that the context element has an expected CSS property. | ||
|
|
||
| The context can be set by the <<_change_context,corresponding steps>>. If no context is set, the text will be searched on the whole page. |
There was a problem hiding this comment.
we do not search text in this step
Comment on lines
552
to
555
| Then the context has a width of '$widthInPerc'% | ||
| ---- | ||
|
|
||
| * `$widthInPerc` - An expected element width in a percentage. |
There was a problem hiding this comment.
Suggested change
| Then the context has a width of '$widthInPerc'% | |
| ---- | |
| * `$widthInPerc` - An expected element width in a percentage. | |
| Then the context has a width of '$widthPercentage'% | |
| ---- | |
| * `$widthPercentage ` - An expected element width in a percentage. |
|
|
||
| Checks that the context element has an expected *width in a percentage* (from the style attribute) relative to the parent element | ||
|
|
||
| The context can be set by the <<_change_context,corresponding steps>>. If no context is set, the text will be searched on the whole page. |
There was a problem hiding this comment.
no text search in this step
Comment on lines
578
to
581
| Then the context element has a width of '$widthInPerc'% relative to the parent element | ||
| ---- | ||
|
|
||
| * `$widthInPerc` - An expected element width in a percentage. |
There was a problem hiding this comment.
Suggested change
| Then the context element has a width of '$widthInPerc'% relative to the parent element | |
| ---- | |
| * `$widthInPerc` - An expected element width in a percentage. | |
| Then the context element has a width of '$widthPercentage'% relative to the parent element | |
| ---- | |
| * `$widthPercentage ` - An expected element width in a percentage. |
053df57 to
400518e
Compare
uarlouski
reviewed
Aug 30, 2023
| Then each element located `id(context-menu-item)` has same `height` | ||
| ---- | ||
|
|
||
| === Check element width in a percentage |
There was a problem hiding this comment.
in this case the width is calculated relatively to body, please add this in docs
uarlouski
reviewed
Aug 30, 2023
| File fileForUpload = ResourceUtils.isFileURL(resource.getURL()) ? resource.getFile() | ||
| : unpackFile(resource, filePath); | ||
| B83A | Validate.isTrue(fileForUpload.exists(), FILE_EXISTS_MESSAGE_FORMAT, filePath); | |
| String fullFilePath = fileForUpload.getAbsolutePath(); |
There was a problem hiding this comment.
inline or bring it closed to the place where its used?
edaa711 to
69d0643
Compare
uarlouski
approved these changes
Aug 31, 2023
69d0643 to
8bc8aa5
Compare
valfirst
approved these changes
Sep 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.