-
-
Notifications
You must be signed in to change notification settings - Fork 90
Add step to verify element CSS properties #5586
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
8ca7cdb
to
e6ac923
Compare
489be47
to
c36fc43
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5586 +/- ##
=========================================
Coverage 97.69% 97.70%
- Complexity 7124 7150 +26
=========================================
Files 987 992 +5
Lines 20674 20755 +81
Branches 1353 1353
=========================================
+ Hits 20198 20279 +81
Misses 364 364
Partials 112 112 ☔ View full report in Codecov by Sentry. |
|
||
[source,gherkin] | ||
---- | ||
Then context element has CSS properties:$parameters |
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.
Then context element has CSS properties:$parameters | |
Then context element has CSS properties matching rules:$parameters |
---- | ||
|
||
* `$parameters` - The xref:ROOT:glossary.adoc#_examplestable[ExamplesTable] containing the following columns: | ||
** [subs=+quotes]`*cssName*` - A name of the CSS property. |
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.
** [subs=+quotes]`*cssName*` - A name of the CSS property. | |
** [subs=+quotes]`*cssProperty*` - A name of the CSS property. |
<meta charset="utf-8"> | ||
<title>CSS properties validation results table</title> | ||
<link rel="stylesheet" href="../../styles.css"/> | ||
<link rel="stylesheet" href="../../webjars/bootstrap/3.4.1/css/bootstrap.min.css"/> |
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.
please use bootstrap 5 (it's also available in webjars)
* @param parameters The parameters of the expected CSS properties to set as ExamplesTable | ||
*/ | ||
@Then("context element has CSS properties:$parameters") | ||
public void doesElementHasCssProperties(ExamplesTable parameters) |
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.
does it make sense to use POJO instead of ExamplesTable
?
@@ -0,0 +1,34 @@ | |||
Description: Integration tests for ElementSteps class. |
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.
Description: Integration tests for ElementSteps class. | |
Description: Integration tests for features working with CSS properties. |
* | ||
* @param parameters The parameters of the expected CSS properties to set as ExamplesTable | ||
*/ | ||
@Then("context element has CSS properties:$parameters") |
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 it makes sense to move this step ElementCssSteps
class (to keep consistency with Selenium plugin)
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.
done
[source,gherkin] | ||
---- | ||
When I change context to element located by `id(block)` | ||
Then context element has CSS properties: |
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.
as practice shows steps that consume locator are more flexible, e.g. Then element located by ... has CSS properties..., if you need to check against context then you can just pass xpath(.) locator
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.
to discuss
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.
doubtful at the current moment
* @param parameters The parameters of the expected CSS properties to set as ExamplesTable | ||
*/ | ||
@Then("context element has CSS properties:$parameters") | ||
public void doesElementHasCssProperties(ExamplesTable parameters) |
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.
does have :)
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.
ToDo
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.
fixed
StringComparisonRule comparisonRule = params.valueAs("comparisonRule", StringComparisonRule.class); | ||
|
||
String actualCssValue = getCssValue(elementCss, cssName); | ||
boolean passed = softAssert.assertThat(String.format(ELEMENT_CSS_CONTAINING_VALUE, cssName, expectedValue), |
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.
ELEMENT_CSS_CONTAINING_VALUE, its not only containing as you use comparison rule
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.
ToDo
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.
fixed
Map<String, String> values = params.values(); | ||
String cssName = values.get("cssName"); | ||
String expectedValue = values.get("expectedValue"); | ||
StringComparisonRule comparisonRule = params.valueAs("comparisonRule", StringComparisonRule.class); |
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.
does it make sense to map params directly to pojo having css name, exp val and comp rule?
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.
fixed
e3f93c6
to
8fa473a
Compare
|
||
import org.vividus.steps.StringComparisonRule; | ||
|
||
public class CssValidationResult extends CssValidationParameters |
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.
use composition over inheritance
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.
fixed
|
||
[source,gherkin] | ||
---- | ||
Then context element does have CSS properties matching rules:$parameters |
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.
why does have
but not has
?
<table class="table table-hover table-bordered table-condensed fixedHeader"> | ||
<thead> | ||
<tr> | ||
<th>Css Property</th> |
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.
<th>Css Property</th> | |
<th>CSS Property</th> |
<th>Actual Value</th> | ||
<th>Comparison Rule</th> | ||
<th>Expected</th> | ||
<th>Result</th> |
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.
the title for this column can be omitted and it makes sense to make this column first
public void doesElementHasCssProperties(List<CssValidationParameters> parameters) | ||
{ | ||
String getAllCssScript = | ||
org.vividus.util.ResourceUtils.loadResource("org/vividus/ui/web/get-element-computed-css-func.js"); |
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.
just use import instead of fully-qualified name
boolean passed = softAssert.assertThat(String.format("Element has CSS property '%s' %s value '%s'", | ||
cssName, comparisonRule, expectedValue), |
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.
boolean passed = softAssert.assertThat(String.format("Element has CSS property '%s' %s value '%s'", | |
cssName, comparisonRule, expectedValue), | |
boolean passed = softAssert.assertThat(String.format("CSS property '%s' value", | |
cssName), |
private List<CssValidationResult> validateElementCss(List<CssValidationParameters> parameters, | ||
Map<String, String> elementCss) | ||
{ | ||
List<CssValidationResult> cssResults = new ArrayList<>(); | ||
parameters.forEach(param -> | ||
{ | ||
String cssName = param.getCssProperty(); | ||
String expectedValue = param.getExpectedValue(); | ||
StringComparisonRule comparisonRule = param.getComparisonRule(); | ||
|
||
String actualCssValue = getCssValue(elementCss, cssName); | ||
boolean passed = softAssert.assertThat(String.format("Element has CSS property '%s' %s value '%s'", | ||
cssName, comparisonRule, expectedValue), | ||
actualCssValue, comparisonRule.createMatcher(expectedValue)); | ||
cssResults.add(new CssValidationResult(param, expectedValue, passed)); | ||
}); | ||
return cssResults; | ||
} | ||
|
||
private String getCssValue(Map<String, String> cssMap, String cssName) | ||
{ | ||
return Optional.ofNullable(cssMap.get(cssName)).orElseGet(() -> { | ||
String cssValueAsCamelCase = CaseUtils.toCamelCase(StringUtils.removeStart(cssName, '-'), false, '-'); | ||
return cssMap.get(cssValueAsCamelCase); | ||
}); | ||
} |
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 makes sense to introduce CssValidations
class, move this logic to it and re-use in both places
055e186
to
99357d4
Compare
vividus-tests/src/main/resources/story/integration/ElementCssStepsTests.story
Show resolved
Hide resolved
d6156f1
to
06e2c7b
Compare
6fb528b
to
1c4eb54
Compare
No description provided.