-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI: Fix retry action #306
CI: Fix retry action #306
Conversation
Confirmed |
Okay it exited with code 0 even though I passed a command that should fail, I think because of a bug in the logic about attempt count |
Okay the https://github.com/mne-tools/mne-lsl/actions/runs/10253578557/job/28366526502?pr=306 |
Appears to have worked on the Pytest step, too.
I guess maybe you also want exit code 1 in |
Appears to have worked as this one took two attempts: https://github.com/mne-tools/mne-lsl/actions/runs/10253825291/job/28367329623?pr=306#step:8:5621 |
|
Okay look at 18c2a49 then it had one or two jobs fail with exit 1 |
Like this run specifically https://github.com/mne-tools/mne-lsl/actions/runs/10253602827/job/28366606652 |
Maybe those tests could use https://pypi.org/project/pytest-retry/ since they're flaky but don't |
Yes, in this one, an LSL stream from other tests was not properly terminated.. Probably one of the rarest flakiness. Anyway, the idea was to have this small action to recover from hard crashes; and eventually |
But this run https://github.com/mne-tools/mne-lsl/actions/runs/10253602827/job/28366606652#step:8:14480 now suggest that when an hard crash occurs, LSL streams are not properly terminated and are still lingering in the background, making the test suit fail on the next iteration when looking for a fix number of streams with |
Closes #301
First part of the suggestion from #301 (comment). Let's see if the default is to have errorexit on.