microbit-foundation / python-editor-v3

Micro:bit Educational Foundation Python Editor V3
https://python.microbit.org
MIT License
54 stars 36 forks source link

Flaky e2e test: open > Correctly handles an mpy file #1110

Closed microbit-matt-hillsdon closed 1 year ago

microbit-matt-hillsdon commented 1 year ago

This test regularly fails when run in GitHub actions. This wasn't an issue in CircleCI, or at the least not so frequent.

Running 100s of times in a loop locally, I can't get it to fail.

For example, this build.

    Evaluation failed: Error: Found multiple elements with the text: This version of the Python Editor doesn't currently support adding .mpy files.

    Here are the matching elements:

    <div
      class="chakra-alert__desc css-zycdy9"
    >
      This version of the Python Editor doesn't currently support adding .mpy files.
    </div>

    <div
      class="chakra-alert__desc css-zycdy9"
    >
      This version of the Python Editor doesn't currently support adding .mpy files.
    </div>

We dump a screenshot after the failure which only shows a single toast:

open-Correctly-handles-an-mpy-file

This is the only test that would trigger that error message.

Debugging the app when openg the test file locally, it inserts the alert, sets a timeout then removes the toast. At no point can I see another copy in the DOM, at least in the subtree the toast manager uses.

If we triggered the error twice then this could make sense, as the toasts stack. But to compound my confusion, we go out of our way to close any open notifications when we load a new file.

We could relax what's being asserted, but it makes no sense to me.

microbit-matt-hillsdon commented 1 year ago

If you run this absolute evil:

setInterval(() => { const p = Array.from(document.querySelectorAll(".chakra-alert__desc")); p.length > 1 && console.log(p.map(p => p.parentElement.parentElement.parentElement.parentElement)) }, 100)

...then you can see that there are two of them and one of them is in an aria-live region. So it does make sense!

microbit-matt-hillsdon commented 1 year ago

This post-fix job failed in a somewhat similar way (though a different test). I don't think it can be quite the same issue as of the two previous matches only one was actually role=alert.

https://github.com/microbit-foundation/python-editor-v3/actions/runs/4331196449/jobs/7562883041

Screenshot only shows one toast (though it'll be a few millis later).

The alerts are literally identical with the same id="1". I guess I'm now tempted to make it work regardless of other alerts.