garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.66k stars 602 forks source link

Ready event seems to be unreliable #1563

Open lpeabody opened 3 months ago

lpeabody commented 3 months ago

What are some scenarios where the ready event always fires but some scenarios still log the following:

ReadyEvent not detected within readyTimeout limit. (30000 ms) ...

It's inconsistent, so it seems like maybe the ready event fires too quickly, perhaps?

lpeabody commented 3 months ago

I'm unsure just by reading the source, but, it appears that BackstopTools need to be installed before the ready event is logged to the console?

I'm finding this to be helpful:

// Let's promise to resolve that BackstopTools is installed before outputting the ready event to the console.
let backstopToolsInstalledResolve;
const backstopToolsInstalledPromise = new Promise(resolve => {
  backstopToolsInstalledResolve = resolve;
});
const backstopReadyWhenAllResolved = [
  backstopToolsInstalledPromise
];

...

// Give it up to 10 seconds to become ready.
let readyEventIntervalCounter = 0;
const readyEventInterval = setInterval(() => {
  if (window._readyEvent) {
    backstopToolsInstalledResolve();
    clearInterval(readyEventInterval);
  }
  if (readyEventIntervalCounter++ > 10) {
    clearInterval(readyEventInterval);
  }
}, 1000);

// When all required promises to be ready have been fulfilled, log the event.
// Also wait a few seconds for good measure.
Promise.all(backstopReadyWhenAllResolved)
  .then(() => {
    setTimeout(() => {
      console.log(window._readyEvent);
    }, 3000);
  });

I think it would be helpful to suggest this as a recipe in the documentation, otherwise using the ready event feature may seem like it's not working as intended and its not obvious at all as to why.

EDIT: If you think it would be useful I can setup a PR with the added documentation.