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

Add document and change the syntax on the Script Steps #4612

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

TykovkaV
Copy link
Contributor

No description provided.

@TykovkaV TykovkaV requested a review from a team as a code owner November 28, 2023 14:39
@TykovkaV TykovkaV force-pushed the change_script_steps branch from 158ce16 to 2b2d4f9 Compare November 28, 2023 14:44
Copy link
Collaborator

@valfirst valfirst left a comment

Choose a reason for hiding this comment

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

Make sure to deprecate the steps in the code

@@ -1114,6 +1114,64 @@ When I click on a button with the name 'Toggle element visibility with delay'
Then element located by 'xpath(//div[@id='delayed'])' disappears in 'PT3S'
----

==== Script Steps

WARNING: These steps are deprecated and will be removed in VIVIDUS 0.8.0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
WARNING: These steps are deprecated and will be removed in VIVIDUS 0.8.0.
WARNING: These steps are deprecated and will be removed in VIVIDUS 0.7.0.

The Script Steps checks for validate the presence of JavaScript files within the HTML source code of an active page.
These steps primarily focus on verifying the existence of specific JavaScript files based on Filename, Text and TextPart attributes.

=== The script steps on FileName attributes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

😵‍💫

@TykovkaV TykovkaV requested a review from valfirst December 11, 2023 11:15
@TykovkaV TykovkaV force-pushed the change_script_steps branch from fe59818 to d88ac89 Compare December 13, 2023 10:28
.Checks the script for the presence of a document by name
[source,gherkin]
----
Then a javascript file with the name '$jsFileName' is included in the source code
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace placeholders for step parameters in examples here and below

@TykovkaV TykovkaV force-pushed the change_script_steps branch from d88ac89 to 10ea3bb Compare December 13, 2023 11:37
@TykovkaV TykovkaV requested a review from draker94 December 13, 2023 11:37
@TykovkaV TykovkaV force-pushed the change_script_steps branch 7 times, most recently from 2404c30 to 1ad7a42 Compare December 18, 2023 22:06
Copy link

codecov bot commented Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3916ac6) 97.40% compared to head (8a6e035) 97.40%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #4612   +/-   ##
=========================================
  Coverage     97.40%   97.40%           
  Complexity     6563     6563           
=========================================
  Files           915      915           
  Lines         18878    18878           
  Branches       1258     1258           
=========================================
  Hits          18389    18389           
  Misses          383      383           
  Partials        106      106           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

*/
@Deprecated(since = "0.6.5", forRemoval = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Deprecated(since = "0.6.5", forRemoval = true)
@Deprecated(since = "0.6.6", forRemoval = true)

*/
@Deprecated(since = "0.6.5", forRemoval = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Deprecated(since = "0.6.5", forRemoval = true)
@Deprecated(since = "0.6.6", forRemoval = true)

*/
@Deprecated(since = "0.6.5", forRemoval = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Deprecated(since = "0.6.5", forRemoval = true)
@Deprecated(since = "0.6.6", forRemoval = true)

@Then("a javascript file with the name '$jsFileName' is included in the source code")
public WebElement thenJavascriptFileWithNameIsIncludedInTheSourceCode(String jsFileName)
{
LOGGER.warn(String.format(DEPRECATED_STEP_MESSAGE, "thenJavascriptFileWithNameIsIncludedInTheSourceCode"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably in these cases it would be be enough to use @Replacement annotation, also please pay attention our log messages are for end-users, not technical people, thus we should never use method names.

@TykovkaV TykovkaV force-pushed the change_script_steps branch from 1ad7a42 to a7a0b1c Compare December 20, 2023 11:36
@TykovkaV TykovkaV requested a review from valfirst December 20, 2023 11:52

=== Validate JavaScript files is present in HTML source code

These step primarily focus on checking for the existence of certain JavaScript files based on FileName attributes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

please follow the same format as for other steps in the docs

Copy link
Collaborator

Choose a reason for hiding this comment

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

the same for all step docs

Comment on lines 1257 to 1258
The Script Steps verify the presence of JavaScript files in the HTML source code of an active page.
They focus on validating specific JavaScript files using attributes like FileName, Text, and TextPart.
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • not only files
  • there is no such attributes as text or text part

.Checks the script for the presence of a document by name
[source,gherkin]
----
Then a javascript file with the name 'app' is included in the source code
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, make examples for more close to real, obviously there couldn't be JS files with name app

Then a javascript file with the name '$jsFileName' is included in the source code
----

* `$jsFileName` - The JavaScript FileName. In order to save a result return statement should be used.
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does it mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

the same for all step docs

@Then("a javascript file with the name '$jsFileName' is included in the source code")
@Replacement(versionToRemoveStep = "0.7.0",
replacementFormatPattern = "Then number of elements found by `xpath(.//script[contains(@src()='%1$s'])` "
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to find all elements regardless of their visibility, check source code of the step to info on the applied visibility attributes

Copy link
Collaborator

Choose a reason for hiding this comment

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

the same for the step below

Copy link
Collaborator

Choose a reason for hiding this comment

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

not fixed

@Then("a javascript with the textpart '$jsTextPart' is included in the source code")
@Replacement(versionToRemoveStep = "0.7.0",
replacementFormatPattern = "Then number of elements found by `xpath(.//script)->filter.textPart('%1$s')` "
Copy link
Collaborator

Choose a reason for hiding this comment

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

why does the form of the locator not match to the value from the step?

@Then("a javascript with the textpart '$jsTextPart' is included in the source code")
@Replacement(versionToRemoveStep = "0.7.0",
replacementFormatPattern = "Then number of elements found by `xpath(.//script)->filter.textPart('%1$s')` "
+ "is greater than or equal to `1`")
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is >=1 ?

@@ -31,7 +33,7 @@
import org.vividus.ui.action.search.Visibility;
import org.vividus.ui.web.action.search.WebLocatorType;

@ExtendWith(MockitoExtension.class)
@ExtendWith({ MockitoExtension.class, TestLoggerFactoryExtension.class })
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant change?

@TykovkaV TykovkaV force-pushed the change_script_steps branch from a7a0b1c to 31ff65a Compare December 20, 2023 14:51
@TykovkaV TykovkaV requested a review from valfirst December 20, 2023 14:52
@TykovkaV TykovkaV force-pushed the change_script_steps branch from 31ff65a to ba32e74 Compare December 29, 2023 09:57
@Then("a javascript file with the name '$jsFileName' is included in the source code")
@Replacement(versionToRemoveStep = "0.7.0",
replacementFormatPattern = "Then number of elements found by `xpath(//script[contains(@src()='%1$s']):a` "
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use exactly the same xpath as in the original code, mind the dot

Comment on lines 81 to 82
* @deprecated Use step:
* This step "Then a javascript with the text '$jsText' is included in the source code" will be removed
Copy link
Collaborator

Choose a reason for hiding this comment

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

plese fix javadocs

Comment on lines 112 to 113
* @deprecated Use step:
* This step "Then a javascript with the textpart '$jsTextPart' is included in the source code" will be removed
Copy link
Collaborator

Choose a reason for hiding this comment

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

plese fix javadocs

@Then("a javascript with the text '$jsText' is included in the source code")
@Replacement(versionToRemoveStep = "0.7.0",
replacementFormatPattern = "Then number of elements found by `xpath(//script[text()='%1$s']):a` "
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use exactly the same xpath as in the original code, mind the dot


=== Check for JavaScript code in HTML source code with a specific file name (@src attribute)

This step verifies that JavaScript (an element with the <script> tag) with specific file name (@src attribute) is present on the page.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow the same approach as for other steps: start with a verb in 3rd form in present simple tense

@@ -1250,6 +1250,63 @@ When I wait until scroll is finished
Then page is scrolled to element located by `id(footerElement)`
----

==== Script Steps

WARNING: These steps are deprecated and will be removed in VIVIDUS 0.7.0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's needed to provide information about replacement for each deprecated step


=== Check for JavaScript code in the HTML source code containing the given text (@script attribute)

This step verifies that JavaScript (an element with the <script> tag) is actual content of text is present on the page.
Copy link
Collaborator

Choose a reason for hiding this comment

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

something broken in the second half of the sentence


* `$jsTextPart` - Specifies the text of an external script file from the script attribute.

.Checks if the javascript file containing the given text exists in the page source code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not correct


* `$jsText` - Specifies the text of an external script file from the script attribute.

.Checks if the javascript file with the given text exists in the page source code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not correct

@TykovkaV TykovkaV force-pushed the change_script_steps branch from ba32e74 to 39f1f2b Compare January 2, 2024 18:02
@TykovkaV TykovkaV requested a review from valfirst January 2, 2024 18:40
@TykovkaV TykovkaV force-pushed the change_script_steps branch from a895535 to a14d545 Compare January 2, 2024 18:55
@TykovkaV TykovkaV force-pushed the change_script_steps branch 2 times, most recently from 096fa58 to 2bb2c7a Compare January 4, 2024 10:50
_Replacement_:
[source,gherkin]
----
Then number of elements found by `xpath(.//script[contains(@src()='%1$s']):a` is equal to `1`
Copy link
Contributor

Choose a reason for hiding this comment

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

what does %1$s mean


==== Check script with the specified content is included on the page

Checks that the page contains a script (an element with the <script> tag) whose content is equal to the specified content.
Copy link
Contributor

Choose a reason for hiding this comment

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

the specified content -> the specified text

_Replacement_:
[source,gherkin]
----
Then number of elements found by `xpath(.//script[text()='%1$s']):a` is equal to `1`
Copy link
Contributor

Choose a reason for hiding this comment

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

Then a javascript with the text '$jsText' is included in the source code
----

* `$jsText` - Specifies the text of an external script file from the script attribute.
Copy link
Contributor

Choose a reason for hiding this comment

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

what does it mean?


==== Check if HTML includes a script that contains the specified content

Checks that the page contains a script (an element with the <script> tag) whose content contains the specified content.
Copy link
Contributor

Choose a reason for hiding this comment

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

specified text

_Deprecated syntax (will be removed in VIVIDUS 0.7.0)_:
[source,gherkin]
----
Then a javascript file with the name '$jsTextPart' is included in the source code
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong step

@TykovkaV TykovkaV force-pushed the change_script_steps branch from 2bb2c7a to bed80dd Compare January 4, 2024 18:05
@TykovkaV TykovkaV requested a review from vkepin January 4, 2024 18:05
@TykovkaV TykovkaV force-pushed the change_script_steps branch from bed80dd to 743314e Compare January 5, 2024 15:25

The Script Steps verify the presence of JavaScript files in the HTML source code of an active page.

==== Check script with the specified file name is included on the page
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
==== Check script with the specified file name is included on the page
==== Check presence of script resource


==== Check script with the specified file name is included on the page

Checks script (an element with the <script> tag) with the specified file name (@src attribute) is included on the page.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Checks script (an element with the <script> tag) with the specified file name (@src attribute) is included on the page.
Checks script (an element with the `<script>` tag) with the specified path (`@src` attribute) is included on the page.


Checks script (an element with the <script> tag) with the specified file name (@src attribute) is included on the page.

WARNING: This step is deprecated and will be removed in VIVIDUS 0.7.0. Please see the replacement below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, move replacement inside this warning admonition


WARNING: This step is deprecated and will be removed in VIVIDUS 0.7.0. Please see the replacement below.

_Deprecated syntax (will be removed in VIVIDUS 0.7.0)_:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
_Deprecated syntax (will be removed in VIVIDUS 0.7.0)_:

no need to duplicate the info

Then a javascript file with the name '$jsFileName' is included in the source code
----

* `$jsFileName` - Specifies the URL of an external script file from the src attribute.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `$jsFileName` - Specifies the URL of an external script file from the src attribute.
* `$jsFileName` - The part of the script resource URL from the `@src` attribute.

Then a javascript with the textpart '$jsTextPart' is included in the source code
----

* `$jsTextPart` - Specifies text that should be contained inside a <script> tag.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `$jsTextPart` - Specifies text that should be contained inside a <script> tag.
* `$jsTextPart` - The expected text to be contained in the text of a `<script>` tag.

Then number of elements found by `xpath(.//script[contains(text(),'jsTextPart')]):a` is equal to `1`
----

.Checks if the javascript text contains 'Hello JavaScript'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.Checks if the javascript text contains 'Hello JavaScript'
.Checks if the JavaScript code contains 'Hello JavaScript'

@@ -46,8 +47,14 @@ public class ScriptSteps
*
* @param jsFileName Value of the <i>src</i> attribute
* @return webElement found js script
* @deprecated Use step:
* "Then number of elements found by `xpath(.//script[contains(@src()='%1$s']):a` is equal to `1`" instead
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* "Then number of elements found by `xpath(.//script[contains(@src()='%1$s']):a` is equal to `1`" instead
* "Then number of elements found by `xpath(.//script[contains(@src()='<jsFileName >']):a` is equal to `1`" instead

@@ -71,8 +78,14 @@ public WebElement thenJavascriptFileWithNameIsIncludedInTheSourceCode(String jsF
*
* @param jsText Content of the <i>{@literal <script>}</i> tag.
* @return webElement found js script
* @deprecated Use step:
* "Then number of elements found by `xpath(.//script[text()='%1$s']):a` is equal to `1`" instead
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* "Then number of elements found by `xpath(.//script[text()='%1$s']):a` is equal to `1`" instead
* "Then number of elements found by `xpath(.//script[text()='<jsText>']):a` is equal to `1`" instead

@@ -96,8 +109,14 @@ public WebElement thenJavascriptFileWithTextIsIncludedInTheSourceCode(String jsT
*
* @param jsTextPart String which should be contained in the <i>{@literal <script>}</i> tag.
* @return webElement found js script
* @deprecated Use step:
* "Then number of elements found by `xpath(.//script[contains(text(),'%1$s')]):a` is equal to `1`" instead
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* "Then number of elements found by `xpath(.//script[contains(text(),'%1$s')]):a` is equal to `1`" instead
* "Then number of elements found by `xpath(.//script[contains(text(),'<jsTextPart.')]):a` is equal to `1`" instead

@TykovkaV TykovkaV force-pushed the change_script_steps branch 2 times, most recently from 19acb69 to 1eba7bb Compare January 8, 2024 15:58
@TykovkaV TykovkaV requested a review from valfirst January 8, 2024 16:02
@TykovkaV TykovkaV force-pushed the change_script_steps branch from de9d510 to 17051b1 Compare January 8, 2024 16:32
@valfirst valfirst merged commit 34035a2 into vividus-framework:master Jan 8, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants