Hi Marcin A few comments inline..
On Saturday, February 8, 2020 at 1:20:29 PM UTC+1, Marcin Erdmann wrote: > > Hi François, > > Thank you very much for your email. It's great to learn that Geb is being > used to test the frontend of Gradle Enterprise. It's also cool to see that > a fairly new addition to Geb in the form of NavigatorEventListener is being > used out there in the wild. > With Luke on the team, what would you expect ;) ? We've been using it right from the start. > > I think what you're doing makes a lot of sense. I think most folks writing > browser tests for modern, single page apps deal with flakiness caused by > async events in one form or another. For example, on my current project, we > recently had a push to quash flakiness in our tests and introduced a number > of custom reporters which turned out to be very effective: > - one which writes browser console logs > - one which remote controls (https://github.com/ldaley/remote-control) into > the server and writes a thread dump to see if any long running operations > are currently in flight > > Both of them could probably be pulled into Geb core in one form or > another, with the biggest stumbling block probably being making the first > one cross browser. > > I find big parts of what you've done useful and generic enough to be > pulled into core. Let me comment one by one: > > 1. I don't see much value in adding the window size onto the screenshot. I > personally tend to make my tests reliable with regards to browser size by > always setting the browser size to a predefined value when constructing a > driver instance, e.g.: > > driver.manage().window().size = new Dimension(1024, 768) > We do have such setting, but with large width/height. It seems that when executed locally, if the window dimension is larger than your machine screen, then the dimension is capped to the screen. Therefore we sometimes have different behaviors locally on and CI (e.g. tooltip hiding an element that is then not clickable, due to the positioning of the tooltip being somewhat influenced by the window dimension). > > 2. For reporting browser url I would probably introduce a > new geb.report.Reporter implementation that writes the url into a text > file, I don't think that there is a valid reason for why that information > should be written onto the screenshot > Agreed. This was a quick hack (yet valuable) to pinpoint some missing waitFor's. That being said, having to look at a text file _and_ a screenshot is less ideal to get the global 'context' of a browser test failure. > > 3. Writing out current active element and mouse position looks great. We > could enhance geb.report.ScreenshotReporter with configuration properties > like reportCurrentElement and reportMousePosition and write that info into > the screenshot if these properties are set - that way it would be a > backwards compatible change and we could change the default values of these > props to true in the next major version. Unfortunately I don't have the > capacity at the moment to see if one can get mouse position in a cleaner or > more efficient way than you have done. > > 4. For dealing with rendering the position of what will be clicked I think > we should extend the reporting mechanism to be able to pass additional > information via some kind of a context and then react in the reporter to > that data being set and perform additional actions. > > If you’re up for contributing back what you have then I’d suggest > attacking points 2 and 3 and when we get that merged I will tackle 4 cause > that will be a bit more involved with regards to introducing a reporting > context. > I'll try to prepare something on my time off. It's therefore going to take a bit of time, but I'll do it. While we're discussing improvements, it would be great than the 'context' you're referring to was shared between the beforeClick / afterClick callbacks., and if we could somehow inject an arbitrary object into it. The use case is that we have some clicks that trigger an animation (using velocity.js). The targeted element has a 'velocity-animating' CSS class when being animated. We currently use the presence (or the lack thereof) of this CSS class to ensure the animation is fully done. But the logic is not perfect, because if we're entering the 'afterClick' callback before the animation has even started (which could happen when the browser is under stress, like in CI), we don't find the 'velocity-animating- CSS class, and think the animation is over. To overcome this, I still need to have a little waitFor with a timeout, to ensure sufficient time is waited (for the likeliness of this scenario to happen to be near zero) If we share an object between the beforeClick/afterClick context, I can then register something that somehow is notified when the animation will start (and we know it will start), and then the afterClick can query this same object to know if the animation has started (and potentially is already over) reliably. > > Cheers, > Marcin > > On Wed, Feb 5, 2020 at 10:15 PM François Guillot <[email protected] > <javascript:>> wrote: > >> I actually forgot to add the screenshots mentioned... >> >> >> >> On Wednesday, February 5, 2020 at 11:14:41 PM UTC+1, François Guillot >> wrote: >>> >>> Hi, >>> >>> We are using Geb, associated to a ChromeDriver instance to execute a >>> bunch of browser tests on our application. >>> Some of those tests are flaky, mainly due to the lack of 'waitFor {}' >>> calls around animations that happen when clicking a web element. >>> >>> In order to help us understanding and fixing the flakiness, we capture >>> screenshots when a test fails. >>> But this is sometimes not enough. >>> >>> Say, the test fail with 'waitFor { someElement.displayed }', the >>> screenshot will either show >>> - that the element is indeed not displayed >>> - that the element is displayed, but that doesn't help. It might be that >>> by the time the screenshot is captured, the animation is actually finished >>> and the screenshot doesn't really reflect the _exact_ state when the >>> failure occurred. >>> >>> >>> So, in order to assist even more, I had the idea to enhance the >>> screenshots with some data. >>> Basically, I added a 'ReportingListener', that grabs the created PNG >>> screenshot, and writes a bunch of stuff on top if it, using java's >>> Graphics2D API. >>> Currently, I add info like: >>> - the browser dimension (it can help in case of tooltips being displayed >>> and hiding elements between them. These tooltips positions can differ based >>> on the browser's dimension) >>> - the browser URL >>> - the current 'document.activeElement' >>> - the current cursor position. >>> - since we can't get the mouse position directly, but only when the >>> mouse moves (correct me if I'm wrong), I hack around this by adding a >>> 'document.onmousemove >>> = event => { window.mouseX = event.pageX; window.mouseY = event.pageY' >>> script, and then do 'browser.interact { moveByOffset(1, 0) }', and then >>> execute 'return [window.mouseX, window.mouseY]', and substract 1 to the >>> mouseX returned value. >>> - I also capture the cursor position before and after click >>> - I added a NavigatorEventListener >>> - implement 'beforeClick' / 'afterClick' and use the same approach as >>> above to store the Navigator.x / Navigator.y values in some variables to be >>> picked up by the 'ReportingListener' >>> - This helps a lot in case where clicking an element triggers an >>> animation that moves the element being clicked. >>> See for instance this page >>> <https://scans.gradle.com/s/qxtasxn3dikjc/timeline>, and click the >>> magnifying glass. >>> In theory, if our 'waitFor { }' were perfect, the 'afterClick' >>> cursor location should still be on the magnifying glass (that has moved >>> vertically after the 'slide down' transition). But the screenshots now show >>> that this is not the case, bringing an actual proof that the waits are not >>> properly working. They just don't properly wait for the end of the >>> animation. >>> >>> This can be seen in the two attached screenshots : >>> - the cursor is correctly on the top-left corner of the magnifying glass >>> on the 'beforeClick' image >>> - in the 'afterClick' one, the cursor has slightly moved vertically, >>> following the slide down transition, but it actually is not on the top-left >>> corner of the magnifying glass >>> >>> Does this sound like a sensible approach to you? >>> Would it make sense that I work on making a PR to integrate that in some >>> sort of ways in Geb? >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "Geb User Mailing List" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/geb-user/4042ad52-757e-41b9-be2a-9bee2639623e%40googlegroups.com >> >> <https://groups.google.com/d/msgid/geb-user/4042ad52-757e-41b9-be2a-9bee2639623e%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > -- You received this message because you are subscribed to the Google Groups "Geb User Mailing List" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/geb-user/0c7b7f3a-164b-49fe-865f-7ac776f4eb22%40googlegroups.com.
