Re: [geb-user] Re: Enhanced screenshots to help understand flakiness
Le dim. 16 févr. 2020 à 14:58, Marcin Erdmann a écrit : > Hi François, > > Apologies for the delay in my response, I was on holidays. > > On Sat, Feb 8, 2020 at 1:34 PM François Guillot > wrote: > >> With Luke on the team, what would you expect ;) ? We've been using it >> right from the start. >> > > You never know, people move on and find new toys to play with. :) > > >> 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). >> > > Right, I see. > We might decide to cap the browser dimension to a dimension lower than a standard 15" laptop screen though, to get around the problem. > > >> 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. >> > > I agree with you that it's more convenient to have that info all in one > file, but we have to be careful when considering pulling stuff into core > not to make the screenshots a dumping ground for information that might be > useful to some users. As I said, I'd be more than happy to accept a PR with > a custom reporter which reports on the url and dimensions of the browser in > a text form but I'm afraid I will have to say no to including that > information in the screenshot as I personally believe it is not the right > place for that information. > Totally agree with you here. If it ends up in core, it will be on a separate report. > > >> 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. >> > > Looking forward to it. Fingers crossed that you will get round to it at > some point, however long that might take. > > >> 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. >> > > The context I talked about in my previous email was meant to be used to > communicate between a caller to report() and various reporters. What you > are talking about seems to be a completely different concern which has to > do with communicating between respective before/after methods > on NavigatorEventListener and possibly also between beforeAtCheck > and onAtCheckSuccess/onAtCheckFailure methods on PageEventListener. This > sounds like it might be something useful - please feel free to create an > issue for implementing this in the tracker at > https://github.com/geb/issues/issues. > Yes that is exactly that. > On the other hand I'm not sure I understand how this helps you to work > around the fact that there is a period of time between something being > clicked and the animation marker class being added to the animated element > but maybe I don't fully grok what you're trying to explain or do not have > the full context to be able to do so. In my opinion one cannot reliably > wait for an asynchronously initiated (the fact that it's asynchronous being > the key here), temporary state to complete because you either run into the > issue that you will consider the state to be completed because you check > for it before it even started or you run into the possibility of not > detecting the state (animation) to occur
Re: [geb-user] Re: Enhanced screenshots to help understand flakiness
Hi François, Apologies for the delay in my response, I was on holidays. On Sat, Feb 8, 2020 at 1:34 PM François Guillot wrote: > With Luke on the team, what would you expect ;) ? We've been using it > right from the start. > You never know, people move on and find new toys to play with. :) > 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). > Right, I see. > 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. > I agree with you that it's more convenient to have that info all in one file, but we have to be careful when considering pulling stuff into core not to make the screenshots a dumping ground for information that might be useful to some users. As I said, I'd be more than happy to accept a PR with a custom reporter which reports on the url and dimensions of the browser in a text form but I'm afraid I will have to say no to including that information in the screenshot as I personally believe it is not the right place for that information. > 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. > Looking forward to it. Fingers crossed that you will get round to it at some point, however long that might take. > 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. > The context I talked about in my previous email was meant to be used to communicate between a caller to report() and various reporters. What you are talking about seems to be a completely different concern which has to do with communicating between respective before/after methods on NavigatorEventListener and possibly also between beforeAtCheck and onAtCheckSuccess/onAtCheckFailure methods on PageEventListener. This sounds like it might be something useful - please feel free to create an issue for implementing this in the tracker at https://github.com/geb/issues/issues. On the other hand I'm not sure I understand how this helps you to work around the fact that there is a period of time between something being clicked and the animation marker class being added to the animated element but maybe I don't fully grok what you're trying to explain or do not have the full context to be able to do so. In my opinion one cannot reliably wait for an asynchronously initiated (the fact that it's asynchronous being the key here), temporary state to complete because you either run into the issue that you will consider the state to be completed because you check for it before it even started or you run into the possibility of not detecting the state (animation) to occur at all if it is very short and it manages to start and stop between your polling checks. Marcin -- 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 geb-user+unsubscr...@googlegroups.com. To view this discussion on the web visit
