search

apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.8k stars 13.87k forks source link

chore: alter scripts/cypress_run to run one file per command + retry #30397

Closed mistercrunch closed 1 month ago

mistercrunch commented 1 month ago

title ^^^

Simply altering the existing python scripts that oversee/wraps the cypress command. The script before was simply figuring out which part of the matrix it had to run, and running a single command running for a set of test files.

Now I changed it to run one command per test file, and built in retry logic in there for that unit of work (1 test file).

Somehow cypress retries don't trigger when they matter most (when the browser process crashes). The new approach allows for more atomic execution and retries, and probably forces a bit of a cleanup as processes end.

mistercrunch commented 1 month ago

error here seems unrelated to memory

Screenshot 2024-09-25 at 6 09 42 PM
mistercrunch commented 1 month ago

Trying retries

mistercrunch commented 1 month ago

Another run, another failure. Also note that apparently we do have global retries here. Next step I'll try to bump the logging level

Screenshot 2024-09-26 at 9 17 55 AM
mistercrunch commented 1 month ago

trying os.environ["DEBUG"] = "cypress:*"

mistercrunch commented 1 month ago

Ok, now testing/confirming with test designed to fail that it does indeed fail and bubble up. After confirming that, I'll re-run without the test designed to fail and merge.

rusackas commented 1 month ago

There's just one thing that might be worth consideration before merging. If I'm reading this right, any test that fails (after retries) will exit the whole Cypress_run script. This differs from normal Cypress behavior, where all test are run, and multiple failures are allowed to happen.

Think there's a good way to log the failure, keep running tests, and then fail CI at the end like Cypress does today? Just might help people troubleshoot real errors more easily to see the whole pile of failures rather than addressing 'em one at a time.

mistercrunch commented 1 month ago

This "cypress runner" script is only used for CI where a single error is too many. I could change it to keep running the following files and bubble up the error at the end, but failing fast here is probably desirable as the contributor would identify the error faster, and do their homework of fixing the file. It wouldn't be hard to improve this script in various ways, but given that all PRs and all master are plague by the issue I suggest we rush merging this.

rusackas commented 1 month ago

Done and done :) Thanks for the work here!

mistercrunch commented 1 month ago

Here's a fix to a problem introduced in this PR: https://github.com/apache/superset/pull/30429