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
Handling PNG Screenshots is slow #3932
Comments
Thank you for your very detailed research. I want to add some notes to it:
|
Thank you for your recognition. For some of your points i have follow up questions. Also i made experiences with testcafe around screenshot taking and browser setup on different levels(rationale, design, ...), which may be not directly related to this issue here. Would be ready to compile a list of those. Should i split those immediately into issues, enhancements or SO questions? Any third option or preference? |
I think we can discuss all your questions in this thread since it will help us understand the issue better. |
0.1 What it is about: Visually testing a literally single page application(see image 3) that reacts per CSS different to the following browser setups
0.2 The application is really a single HTML page with sections summing up to multiples of 0.3 Target browsers are chrome and firefox(FF), as i prefer to be bound to desktop browsers available on Linux(so no Mac or Windows). Also as cloud based CIs for Windows and Mac are not so commonly supported and you have to take extra care for those in your project setup e.g. regarding file paths. 0.4 Additionally, the project for the app has have two different Delivery Modes: Dev Mode and Prod Mode. Dev Mode is based on webpack-dev-server with hot reload. Prod Mode is physically compiled and served from a single dir. Beyond Dev Mode being served by a quite different base than Prod Mode, Dev Mode has a Mode dependent POSTCSS option which shows a visually translucent 32x32px grid in the browser. 0.5 My use case with testcafe: Check for some combinations of
by screenshot to see if the app works visually as expected. 0.6 Now into the course of how my implementation should have run
Currently i never came to the fourth point, as alone the first three were time consuming and did not work as intended. 0.7 Finally i compiled some other observations and questions regarding testcafe, which went into the last chapter 'Other' 1 How To Configure Browser Setup1.1 First takeaway for me was that by only supplying browser CLI arguments ala chrome's 1.2 But on firefox i was first clueless - there is no CLI for emulation mode. So i prepared an own FF profile prior to startup. Along with the automatic removal of the tmp profile(which btw did not work consistently as does testcafe's), I did some amount of copying testcafe's firefox profile handling procedure (Enhancement 1). 1.3 On that way i also converted from package.json driven testcafe parameters to testcafe's programmatic runTestCafe approach. As the browsers array therein was initially not pretty either, i wrapped that in a Builder pattern (Enhancement 2) with my sensible defaults and hiding/harmonizing browser setups e.g. ;(await new ChromeBrowserConfig.Builder().withWidth(600).build()).output() Where 1.4 [Rant] That 1.5 Still, how to set width/height for the media breakpoints in FF were not clear to me. 1.6 To peek how all this is handled with chrome I discovered testcafe's 1.7 Tried to make sense out of that one's documentation at
Whaaaat? For me it looked like, and after researching still is, a complete specification naming disaster followed by a browser vendor interpretation disaster. 1.8 Along i tried to make sense out of 1.9 Out of those two and other references - of whom i would like to highlight https://www.quirksmode.org/blog/archives/2013/12/desktop_media_q.html - i chrocheted a mental model, which boils down to
where this includes the width/height of the eventual scrollbar(s). This one would kick in on CSS media breakpoints defined by 1.10 While the viewport - a special term for a conceptual rectangle area that does not change while zooming, but only somewhat setable via the HTML
contains the root element, like , and does not include the scrollbars. This one kicks on 1.11 So, i have written a length about finding out how make chrome matching CSS Media query boundaries in emulation mode. But what about FF, having no emulation mode? Turns out that only if you run FF via marionettePort(which is tescafe's default of operation),
1.12 Then it calls FF's 1.13 If this conclusion is right, i vote that t.resizeWindow should be aliased renamed (resizeInnerWindow), documented as such and sensible variants (outer,...) should be built into testcafe(Issue 2) 1.14 And so one can expect to have 1.15 But still, will I never have the viewport related media query kicking in FF? To check on that, turned out that resizing at the 0/+1 pixel cases on one of my media query width's, the media query kicked in?!?! 1.16 Turns out that https://www.quirksmode.org/blog/archives/2013/12/desktop_media_q.html also made the following related observation about some desktop browser vendors
1.17 You can observe this effect with your chrome or FF browser by opening 1.18 So, using 2 How to write test2.1 Overall point is that, inside your test, you have no access to the args of the run (the ones used in
2.2 Infos from the run - that is all about args, browser instance and current test are already orderly parsed and evaluated - are held in sync by testcafe and are expected to be properly unit tested. But these infos are not (officially) accessible for the end user, meaning effort of dabbling live code and source code and leads to overall bad practices(code duplication or reaching into not public API). 2.3 So i presume that there are public APIs lacking(Enhancement 3). Let me show you two examples of usage. 2.4 First one, as explained before, is for FF i need to call
2.5 Second one, in my test i do const runArgsBrowser = await evaluateRunArgsBrowser(t)
await resizeToRunInfoDimensions(t, runArgsBrowser) Where const HEIGHT_RUNINFO_RE = /(?:height=)([0-9]+|$)/
const WIDTH_RUNINFO_RE = /(?:width=)([0-9]+|$)/
...
/**
*
* @param {TestController} t
* @return {Promise<RunArgsBrowser>} runArgsBrowser
*/
const evaluateRunArgsBrowser = async function(t) {
/**
* @type {RunArgsBrowser}
*/
const result = {}
const runArgsBrowser = await t.testRun.browserConnection.browserInfo.alias
result.height = evalRegexAsInt(HEIGHT_RUNINFO_RE, runArgsBrowser)
result.width = evalRegexAsInt(WIDTH_RUNINFO_RE, runArgsBrowser)
... As you can see in 2.6 Second example of usage was organizing screenshots into folders respective to the browser instance under test. Could not employ testcafe's screenshot pattern for this. Finally dabbled again into testcafe's not public API and modeled a folder path from test and testcase down to the browser instance setup under test, including user agent info(also mostly a duplication of what testcafe already implemented). See following for result
3 How to take screenshot3.1 The overall goal was to take a screenshot of each section. So i implemented it as a command pairs per section of like await scrollTo(t, "body > main > section.section-about")
await takeScreenshotAtRunInfoContext(t, "section-about.png") 3.2 Where 4 Screenshot Image Can Have Smaller Dimensions Than Requested By Browser Setup4.1 What i first observed is that the returned screenshot images did pixelwise not match the desired Media Breakpoint width, but where smaller. First i thought that there is something going wrong and that i have to adjust to programmatically. Then i understood that screenshots were delivered by testcafe without the scrollbars. At least if there is some browser info about an active displayed browser scrollbar, so i think testcafe is accounting for 4.2 I wrote "At least" as i have proof that chrome in emulation mode does display a scrollbar, which seems not to be accounted for as scrollbar. It's overlaying above the display and is shown or not shown according to some chrome intern behaviour. 4.3 For all of this i feel a relevant testcafe documentation is missing(Enhancement 4). 4.4 In the course of this i would like to discuss what the arguments for deciding to not include scrollbars in screenshot were on testcafe's team. 4.5 My argument is that scrollbars are part of the interface, even design(yes, you can style those afaik non-standardized) and definitely user experience. Scrollbars are part of the factual expression of the display. Excluding those from screenshots is like dismissing characters of the log request message after the 100th, just because someone thought her editor's maximal line length is 97. 4.6 I would see both behaviours as options in the screenshot API(Enhancement 5). 5 Screenshot API For Fullscreen5.0.1 Here i propose screenshot facilities that screenshots the full page or a selected element, so to screenshot not only what is displayed in the actual browser window's viewport. 5.0.2 As of writing i know that current browsers Chrome and FF support full page screenshots via CDP and Marionette. 5.0.3 FF does it's job pretty well, while totally undocumented. I changed the code in testcafe to use an additional parameter to the FF's screenshot call(lost the code after an update, but may reconstruct it on demand). Got the full page screenshot immediately, and as far as i can say - regarding sticky positioned elements and my multi 5.0.4 Chrome, on the other side, does it's job not very well. First, there is no direct function call, you have to be in Emulation mode and fiddle around with Emulation related calls and parameters(see above described usage of 5.0.5 And second, because of how that is implemented in Chrome, the change of the height done therefore affects the viewport size(sic!), which subsequently affects the browser's 5.0.6 With a height that normally covers all of my sections and what is expected - see image 3. 5.1 My Chrome VH Workaround5.1.0 Workaround i found is only by refactoring the app's css to grind all $accnat-header-height-cf-sm: calc(var(--vh, 1vh) * 100); 5.1.1 So to scheme for further enablement of a full page screenshot in Chrome, just before when i would be about taking a screenshot, i first would measure the actual value of const actualVh = window.innerHeight / 100 5.10 and then set this css variable e.g. document.documentElement.style.setProperty("--vh", actualVh + "px") 5.10 and then evaluate the body's bounding box again. I assert that the height of the body's bounding box before is the same as just taken. 5.11 After taking the screenshot, i unset the css variable document.documentElement.style.removeProperty("--vh") 5.2 Chrome Screenshot/DeviceMetricsOverride Issue Parade5.2.0 Issue 1 There are Chromium Issues about 5.2.1 Issue 2 I observed the same behaviour as Issue 1 when using the option 'Capture full size screenshot' via Chrome Developer Tools. As being not documented, or at least not able without digging deeper, i presume that 'Capture full size screenshot' internally uses 5.2.2 Issue 3 Only when hardware accleration in Chrome is set to off, are screenshots returned reliable(else transparent or black image) and with the intended height(sic!, otherwise see following image; compare that to the expected image 3 above). For this, Chrome may be started into this mode also via the CLI 5.2.3 Issue 4 A pure change via 5.2.4 This issues and workarounds are not proper, and in my situation of the app i feel this comes very close to a show stopper. 5.3 Discussion About Application Of Full Page Screenshot5.1 I am thinking of two applications for screenshots: First application is taking a screenshot as percieved by the user in her browser. This is what the actual screenshot facility is already achieving(while it still takes some care to get at it, see story hitherto). 5.2 Second application is taking a screenshot as precieved by the person responsible for the web design and application. 5.3 As can be seen on Image 1, this does tell you half of the truth(or is it more a third of the truth :). Have to take three screenshots tp cover this section. And would those, side by side, give me the right impression about the appearance of the overall design of those three cards("all fits together in some way")? I do not. 5.4 As application developer, whenever i update the slew of my npm dependencies, i can only hope for the best that that just did not break the appearance of my application. In my special case a single full page screenshot against a reference/gold standard would surfice to verify that. 5.5 Maybe i do want to get a feeling of the sourroundings of the element? Maybe for documentation or presentation purposes? If there is no decent support for full page screenshots, it's like saying a map is useless, because you can not look beyond the forrest. 5.6 Additionally, 6 Other6.1 Document Browser Setup In Relation To Testcafe Behaviours (Enhancement 6)6.1.1 As can be seen in the pargraphs of the Browser Setup and Test chapters, overall testcafe lacks documentation under which condition which behaviour is shown in which browser vendor. 6.1.2 Additionally e.g.
6.1.3 Example 1
6.1.4 Example 2
6.2 Explain Testcafe Under-The-Hood6.2.1 Another point about my understanding of testcafe is that - as one might presume falsely by now speaking at length about Chrome's CDP 1.3(as of writing) or FF's Marionette ?.?(latest, do not know) - testcafe is not just yet another wrapper over those. 6.2.2 Many of the other testcafe commands are routed via a JS proxy(hammerhead) directly into/onto the browser instance running the application under test. In the whole documentation of testcafe this point is never mentioned and AFAIK is very unique. 6.2.3 Would love to hear from tescafe folks about their architectural decision process and detailed pros and cons comparing to w3c driver spec or other competitive products. This would be beneficial for decisions about the ins and outs when evaluating or proposing for an adoption. 6.3 Explain Testcafe Supports Only Latest BrowsersAlso lacking is a more prominent section about why testcafe supports only the latest browser 6.4 Evalute Need That Testcafe Delivers ES3 CodeWhy does testcafe deliver transpiled es3 code? 6.5 Provide ESM For User TestTestcafe already uses 6.6 Enhance LoggingTracing what testcafe does while running a test in between is - well - not existing. Proposal is some debug level that shows timestamped console messages like "Starting browser", "Started browser", "Taking screenshot" with proper relation to test and browser instance. 6.7 Make Browser Window Relateable To Current RunIf you debug tests with more than one browser, one can not relate which browser belongs to the current instance of the test. Proper relation with logging chalk color and browser window chrome color, by PID and debugging port ... See also "Enhance Logging". 6.8 Test Report Esp Xunit Flavor Does Tell You Not Enough6.8.1 From the below test report of a contrived test - can you guess which Browser Setup did fail? How does that browser agent identifier( 6.8.2 How can i undefault the folder placement of the error screenshot? If there occur two of them under the same browser agent identifier, they would have overridden each other. 6.8.3 I would not prefer the result of all browsers accumulated into one testcase. I think this is called a parameterized test. So
<testcase classname="Index_Page_Test" name="take_screenshots (Chrome_76.0.3809_Linux_0.0.0_mob#false_dpr#1_tou#false)" >...</testcase>
<testcase classname="Index_Page_Test" name="take_screenshots (Chrome_76.0.3809_Linux_0.0.0_mob#false_dpr#1_tou#true)" >...</testcase> 6.8.4 Also let users define the testsuite name. 6.8.5 Then, user defined properties would be nice e.g. <properties>
<property name="mobile" value="true"/>
<property name="devicePixelRatio" value="2"/>
<property name="touch" value="true"/>
<property name="screenshots" value="/home/joma/entwicklung/design/acc-natours/target/reports-prod/screenshots/Index_Page_Test/take_screenshots/firefox_ubuntu_591x1024_mob#false_dpr#1_tou#false"/>
<property name="osName" value="Ubuntu"/>
<property name="osVersion" value="16.04.6 LTS"/>
<property name="browserId" value="Qs4wd5"/>
<property name="browserName" value="firefox"/>
<property name="browserVersion" value="68.0.0"/>
...
</properties> 6.8.6 For logging reasons above, it makes sense to grab console error and std out to the report too. 6.8.7 Wondering why is one in the report counted as failure, and the other as error? And should error screenshots be named failure screenshots? <?xml version="1.0" encoding="UTF-8" ?>
<testsuite name="TestCafe Tests: Chrome 76.0.3809 / Linux 0.0.0, HeadlessChrome 76.0.3809 / Linux 0.0.0, Firefox 68.0.0 / Ubuntu 0.0.0, Firefox 68.0.0 / Ubuntu 0.0.0" tests="1" failures="1" skipped="0" errors="1" time="38.697" timestamp="Fri, 02 Aug 2019 19:13:30 GMT" >
<testcase classname="Index_Page_Test" name="take_screenshots (screenshots: /home/joma/entwicklung/design/acc-natours/target/reports-prod/screenshots/Index_Page_Test/take_screenshots)" time="38.628">
<failure>
<![CDATA[
1) AssertionError: expected false to be truthy
Browser: Firefox 68.0.0 / Ubuntu 0.0.0
Screenshot:
/home/joma/entwicklung/design/acc-natours/target/reports-prod/screenshots/Index_Page_Test/take_screenshots/Firefox_68.0.0_Ubuntu_0.0.0/errors/1.png
50 | await scrollTo(t, "body > main > section.section-tours")
51 |
52 | await takeScreenshotAtRunInfoContext(t, "section-tours.png")
53 |
54 | if (getRunInfoCtx(t).runArgsBrowser.isTouchEnabled == false) {
> 55 | await t.expect(false).ok()
56 | }
57 |
58 | await t.hover(
59 | "body > main > section.section-tours > div.row > div:nth-child(1) > div.card",
60 | )
at ok (/home/joma/entwicklung/design/acc-natours/src/tests/index-test.js:55:27)
2) AssertionError: expected false to be truthy
Browser: HeadlessChrome 76.0.3809 / Linux 0.0.0
Screenshot:
/home/joma/entwicklung/design/acc-natours/target/reports-prod/screenshots/Index_Page_Test/take_screenshots/HeadlessChrome_76.0.3809_Linux_0.0.0/errors/1.png
50 | await scrollTo(t, "body > main > section.section-tours")
51 |
52 | await takeScreenshotAtRunInfoContext(t, "section-tours.png")
53 |
54 | if (getRunInfoCtx(t).runArgsBrowser.isTouchEnabled == false) {
> 55 | await t.expect(false).ok()
56 | }
57 |
58 | await t.hover(
59 | "body > main > section.section-tours > div.row > div:nth-child(1) > div.card",
60 | )
at ok (/home/joma/entwicklung/design/acc-natours/src/tests/index-test.js:55:27)
]]>
</failure>
</testcase>
</testsuites> 6.9 Auto Fetch Standard DOM Properties Per HTML Element TypeWhat to auto fetch for eg HTMLImageElement; see /** Provides special properties and methods for manipulating <img> elements. */
interface HTMLImageElement extends HTMLElement {
...
readonly complete: boolean;
readonly naturalHeight: number;
src: string;
srcset: string;
...
} Do not know if there is a performance or memory penalty. 6.10 Debugging Chrome in headless mode should be allowedChrome allows more than one session, so even in headless mode i was in need to use it. I know that this imposes a certain risk, t.i. having debug statements while running the test in CI. Would like to discuss that. 6.11 Support more TS type flowingI know, the following TS hints make this look like complete madness. And it is. But it makes usage so more fool proof. Perhaps testcafe can offer some built-in support for such. /**
* Selects
*
* @param {string} imgSelector
* @return the selected img element by the given imgSelector
*/
const selectImg = async (imgSelector) => {
/** @type { SelectorAPI & HTMLImageElement} */
const result = await /** @type { ? } */ (Selector(
imgSelector,
)).addCustomDOMProperties({
// @ts-ignore
complete: (/** @type {HTMLImageElement} */ el) => {
return el.complete
},
// @ts-ignore
naturalHeight: (/** @type {HTMLImageElement} */ el) => el.naturalHeight,
// @ts-ignore
naturalWidth: (/** @type {HTMLImageElement} */ el) => el.naturalWidth,
// @ts-ignore
currentSrc: (/** @type {HTMLImageElement} */ el) => el.currentSrc,
})
return result
} Usage const about_comp_img_3 = await selectImg(
"body > main > section.section-about > div.row > div:nth-child(2) > div > img.composition__photo.composition__photo--p3",
)
await t.expect(about_comp_img_3.complete).ok()
if (getRunInfoCtx(t).runValuesBrowser.dpr >= 2) {
await t.expect(about_comp_img_3.currentSrc).contains("nat-3-large.")
} else {
await t.expect(about_comp_img_3.currentSrc).contains("nat-3.")
} 6.12 Make display position of testcafe's debug footer configureableHappens that i am testing for browser dimensions that are wider/taller than is available on my monitor. Browsers seem to always open so that their top is within bounds of the screen. As the debug footer being at the bottom, i often have to move the browser window to access to the actions on the debug footer. Which is kind of bothersome. |
We are facing a similar issue, I really recommend putting this on the top of the great testcafe team's priorities. for us sometimes we really need multiple screenshots in one test to show the flow of the user activity. but with the slowness happening it is really not possible for us to do that. |
@joma74 Thank you for your comprehensive report. There is a lot of points to comment, and I will try to post my remarks when I get some time to put my thoughts in order. @devmondo unfortunately, it seems there is no way to speed up taking screenshots other than disabling chrome-less and border-less screenshots. Browser UI elements and scrollbars make visual regression testing harder, and we were asked to implement this functionality. We need to discuss if we want to implement some switches that will urn on and off these features. |
@AndreyBelym thank you very much for the update, i think having an option to disable them is great! we don't always need visual regression and if we do, i think that we as devs should create different CI job for that. |
I also noticed that taking screenshots is time consuming especially for the first screenshot of a test. However this cost is OK for me in regards of the value the screenshots bring to our continuous delivery workflow. Outside of the CI scope, we usually run a single test file (the feature on which we are working) on our local environment. During this use case, screenshots are useless (we see the test happening) but speed is important (we run the test multiple time until the feature is working). This is why we do not enable screenshots thanks to the According to this use case, a good first step would be to fully disable screenshots when they are not enabled. We are currently using v1.5.0 |
@nicgirault thank you for sharing your feedback. We will introduce a more correct way to disable taking screenshots in the next release. |
This issue has been automatically marked as stale because it has not had any activity for a long period. It will be closed and archived if no further activity occurs. However, we may return to this issue in the future. If it still affects you or you have any additional information regarding it, please leave a comment and we will keep it open. |
We're closing this issue after a prolonged period of inactivity. If it still affects you, please create a new issue with up-to-date information. Thank you. |
What is your Test Scenario?
Taking screenshots for visual regression verfication.
What are you suggesting?
Optimize use of png operations
1.1 Identify and minimize costly png operations within testcafe(t.i. reparsing, reading anew from file).
1.2 Forward parsed png within testcafe as in-memory object.
1.1-1.2 E.g. in headless mode
capturer.js
relates tobase.js
for taking the screenshot for chrome and firefox. Inbase.js
it is cropped to dimensions and written to disk. Back tocapturer.js
, the screenshot is read from disk for cropping to dimensions, just to see that nothing needs to be done.1.3 Options to make png handling gradually async. User decides on sync waiting for each screenshot, or callback on all completed or auto complete before test ends at the latest.
1.4 Forward png to testcafe users as in-memory object. E.g. give access to Byte Buffer and reserved file path from
t.takeScreenshot
. Let user decide if screenshot needs to be written to disk.Option to disable thumbnail generation. Beside the personal non-usage and therefore extra time spending, e.g. a thumbnail of this is rendered distorted, so the usage may be non-obvious to others as well.
Out of scope
What alternatives have you considered?
Analyse why upstream
pngjs
takes it's timeEvaluate https://www.npmjs.com/package/node-libpng because of an advertised ~ 4x speed gain
Additional context
First planned step was to let testcafe take screenshots of my multi-100-vvh, nearly static SPA at all it's media breakpoints. Both for firefox and chrome, plus some variatons on different dpr(device-pixel-ratio) and mobile emulations, and all of those against a dev setup and a prod setup.
As of now there is a single test which scrolls to the relevant sections of the web app and takes a screenshot of each. Sums up to 10 screenshots per breakpoint-browser-... combination.
Each breakpoint-browser-... combination is driven by runTestCafe's browser configurations. But after adding my seventh browser setup testcafe became unstable. Screenshots appeared slowly on disk, then halted. Test run never finished, intervened manually by aborting after 15 minutes, left with no status whatsoever.
First thought was that just 7*2 browser instances 'killed' my laptop, but after i 'disabled' the taking of screenshots, all test runs finished successfully within 70 seconds together(dev+prod setup concurrently).
So i was curious and employed clinic js, decreased the browser setup to four, and was pointed to a cpu bound issue. Generated a cpu flame graph, which you can download at https://github.com/joma74/acc-natours/blob/testcafe-pngslow-issue/docs/testcafe-png/clinic/25000.clinic-flame.html.
As you can see, the hot spots are all about pngjs.
Next i sprinkled some
console.time
measurements at base.js and capturer.js. Logged them away and built a report sheet from the data.Key take-away from the data is that more than an overall 3 minutes worth were spent on png read/write access for a total build time of around 2 minutes. Reading or writing a png take each around 1.3 seconds on average. The time spent on png read/write access is proportional to the pixel dimensions of the png.
Additional conclusion is that chrome 75.0.3770 takes twice as long for a screenshot compared to firefox 67.0.0, spending nearly 1 second per screenshot as opposed to 0.5 second respectively on average.
The text was updated successfully, but these errors were encountered: