-
Notifications
You must be signed in to change notification settings - Fork 289
Fix Ubuntu tests often failing first time... #3297
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
Fix Ubuntu tests often failing first time... #3297
Conversation
Signed-off-by: itowlson <[email protected]>
|
I would be really very surprised if my caching changes really caused this: I made them after realizing that we were almost always evicting caches before even using them once. Then again, this is caching. It is CI. It is a combination caching and CI. So who knows! |
|
@tschneidereit I am also surprised. And yet after weeks of Ubuntu testing needing multiple re-runs, I made this change and it completed first time. It could be that using cache disguises some sort of unrelated issue, and the true fix is for that unrelated issue? (Like you say: CI, who knows.) And of it could be that this is not a fix and I just had a freakish run of good luck with these two PRs! I dunno. I just want to not have to do repeated re-run cycles on every push. My suggestion is that unless we either:
then we take this PR and at least see if it helps. |
|
One thing maybe worth doing is to delete the cache entry and retrigger the run. If that also succeeds, I think it's pretty unlikely to be a fluke, and we should definitely land this. I would in general say land it, except I find it so utterly baffling that the act of storing a cache entry would make something pass that didn't before. And looking at the log, as I'd expect there's no cache hit, so the only functional difference here really seems to be that after everything has run, the cache entry is created. |
|
Ooh, except that's not the case, as I realize now, right after sending the previous comment! At the very least, the cache action also sets |
|
Oho! Good find. I've just kicked off the re-run after deleting the cache. Then I can try it without cache but with ETA: #3298 |
|
I understand that this shouldn't work but it tentatively seems like it does. Could we merge it for now in the hope that it helps, even if incorrectly? And then we can do more research on a thing that is more correct when we are not having every PR require multiple re-runs? |
|
Well, my hope was misplaced: https://github.com/spinframework/spin/actions/runs/18451759414/job/52566336947?pr=3290 I shall settle for replacing it with a hope that at least things will be slightly less worse. Anyone got any further ideas? |
…hopefully.
This was the line that seemed to make the difference between green and red in #3296. I'm creating a new PR so as to preserve the #3296 history while we evaluate/test, but also get a cleanish run to reassure myself that this does make the necessary difference.
If it does, @tschneidereit or other GH cache experts need to make a call on whether it's going to mess up our billing or whatever. But the current situation is untenable so if this isn't an acceptable solution then we'll need to figure out what is.