-
Notifications
You must be signed in to change notification settings - Fork 48
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
Screenshots of individual elements of the page #44
Conversation
Update Hide Modifier with find elements by css selector #15
…y using css or xpath element selectors
import org.openqa.selenium.TakesScreenshot; | ||
import org.openqa.selenium.WebDriver; | ||
import org.openqa.selenium.WebDriverException; | ||
import org.openqa.selenium.*; |
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 don't use *
imports.
@@ -22,76 +22,109 @@ | |||
import com.cognifide.aet.job.api.collector.CollectorProperties; |
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 check your formatter settings and make sure you use Google Formatter.
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
# Conflicts: # core/jobs/src/main/java/com/cognifide/aet/job/common/comparators/w3chtml5/W3cHtml5ComparatorJob.java
…ements' into screen_parts # Conflicts: # core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/WebElementsLocatorParams.java
…ements' into screen_parts # Conflicts: # core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/WebElementsLocatorParams.java
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.
Nice work and very valuable feature!
@@ -83,15 +97,62 @@ private boolean isPatternAndResultMD5Identical(byte[] screenshot) { | |||
|
|||
@Override | |||
public void setParameters(Map<String, String> params) throws ParametersException { | |||
//No parameters here | |||
if (StringUtils.isNotBlank(params.get(XPATH_PARAM)) || StringUtils |
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.
Maybe we could move setting parameters from WebElementsLocatorParams
to some method in WebElementsLocatorParams
?
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 is, but this is extra check for screen collector as both of those params are optional.
} | ||
} | ||
|
||
private byte[] bufferedImageToByteArray(BufferedImage bufferedImage) throws ProcessingException { | ||
ByteArrayOutputStream temporaryStream = new ByteArrayOutputStream(); | ||
try { |
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 may get benefit from Closable
interface that ByteArrayOutputStream implements:
try (ByteArrayOutputStream temporaryStream = new ByteArrayOutputStream()){
ImageIO.write(bufferedImage, PNG_FORMAT, temporaryStream);
temporaryStream.flush();
return temporaryStream.toByteArray();
} catch (IOException e) {
throw new ProcessingException("Unable to convert screenshot part to byte Array", e);
}
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.
I always forget about that
| --------- | ----- | ----------- | --------- | | ||
| `xpath` | xpath_to_element | Xpath to element(s) | optional (either xpath or css) | | ||
| `css` | css_selector_to_element | css selector to element(s)| optional (either xpath or css) | | ||
| `timeout` | 1000ms | The timeout for the element to appear, in milliseconds. The max value of this parameter is 15000 milliseconds (15 seconds). | no (default will be used) | |
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 looks that timeout parameter is used only when one of selectors is used. It should be mentioned here unless its possible to update the code (which would be much more awesome!).
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 mean some kind if warning message when timeout param is provided but neither xpath or 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.
I'd rather thought about mentioning at documentation, that timeout
parameter is applicable only when taking a screenshot of a page element. It does not work in fullScreen
mode.
But I would be much more happier if timeout
would also work in fullScreen
mode and there would be no need to change docs then.
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.
But I would be much more happier if timeout would also work in fullScreen mode and there would be no need to change docs then.
I have no idea how to (for what purpose) use timeout param when taking fullscreen-shot
(we have sleep modifier for that)
imo just update doc, what do you think ?
@@ -118,5 +118,19 @@ | |||
<url href="comparators/layout/failed.jsp" /> | |||
</urls> | |||
</test> | |||
|
|||
<test name="comparator-Layout-Partial-SUCCESS"> |
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 provide one more test for xpath
selector.
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.
I've pushed small improvements for this CR.
added ability to take screenshots of parts of the page by using css or xpath element selectors
Please do not merge yet!
(Base branch is set to "modifiers_enhancements" for clear diff until modifiers_enhancements be merged into master branch)
I hereby agree to the terms of the AET Contributor License Agreement.