Skip to content
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

Add workflows to test OCaml trunk on Cygwin #305

Merged
merged 7 commits into from
Mar 14, 2023
Merged

Conversation

shym
Copy link
Collaborator

@shym shym commented Mar 8, 2023

This branch is finally getting mergeable.
Unfortunately, running the test suite on Cygwin takes so much time that it never fits into a single 6-hour run, so this PR proposes to add two workflows, each handling about half of the tests.

Still, I’d like to avoid repeating work as much as possible, in particular building twice the compiler and the dependencies (which take about 50 minutes in all the test rounds I ran...). So the way it is currently split up is:

  • one workflow (Cygwin 1), with its cygwin input variable set to “initializer”, will build the compiler and the dependencies and create a cache entry with all that as soon as they are ready, before starting the tests per se,
  • the other workflow (Cygwin 2), the “waiter”, will sleep for an hour if it does not find the cache entry it hopes for and then retry to fetch from the cache; if it fails again, it will fail the whole run (rather than try to build the compiler after already one hour of timeout lost).

Another possible (if I understood correctly github docs) strategy would be to set up the second workflow as triggered only when the first one completes. It would be slower but it would be simpler and quite possibly more robust (if the two workflows are not started at the same time, for instance when the runners are quite busy, the waiting strategy will fail).
@jmid: do you have any preference between those two options?

shym added 6 commits March 8, 2023 19:34
The path and file names are enough
The test suite takes too much time to run under Cygwin so split it up in
two parts so that it can run within the timeout bound
The two CI workflows have different roles, so that only one should
hopefully build the compiler from scratch, the other one should get it
from cache
@jmid
Copy link
Collaborator

jmid commented Mar 9, 2023

First of all: thanks for this well-structured and carefully prepared PR 🙏
It is really great to be able to include a Cygwin CI target!

do you have any preference between those two options?
I may have a slight preference for the second option (which the documentation CI flow may actually use under neath the hood). However I don't think that should hold us back from merging this PR.

I suppose it is the Unix-emulation of Cygwin (and perhaps some inefficiencies in there) that is causing this difference in execution speed? I hope that we at some point will get profiling tools to figure it out...

Anyway: How can we get a nice CI status badge of the output into the README? For the first/second or both?

Unfortunately, as the test suite is split up into two workflows on
Cygwin, we need two badges
@shym
Copy link
Collaborator Author

shym commented Mar 13, 2023

I may have a slight preference for the second option

I had a go at this option, as an attempt to get one badge for both workflows (on my branch cygwin-7) but I must have missed something in the documentation: the second workflow didn’t trigger when the first one finished.

I suppose it is the Unix-emulation of Cygwin (and perhaps some inefficiencies in there) that is causing this difference in execution speed?

I suppose so. Cygwin has clearly a lot of work to do to map POSIX expectancies on Windows API.

Anyway: How can we get a nice CI status badge of the output into the README? For the first/second or both?

I finally added both in a new commit as I failed two attempts (cygwin-6 and cygwin-7) to create one workflow whose final result would cover the complete test suite. I added them on a new line, since they are trunk-only. But as the lines are already overflowing, they might just as well be added to the trunk line?

@jmid
Copy link
Collaborator

jmid commented Mar 14, 2023

The CI failures were unrelated:

  • 1 flaky GitHub actions
  • 2 flaky Thread mode tests (1 Win bytecode Queue test failing unexpectedly, 1 Cygwin CList int64 negtest taking forever)

I'll therefore merge.

@jmid jmid merged commit a493a29 into ocaml-multicore:main Mar 14, 2023
@shym shym deleted the cygwin-5 branch March 14, 2023 17:22
@jmid
Copy link
Collaborator

jmid commented Mar 21, 2023

The #311 CI workflow for Cygwin 2 failed with

Run actions/cache/restore@v3
Error: Failed to restore cache entry. Exiting as fail-on-cache-miss is set. Input key: 3cac9465be888e4a5895ab22ca2853b18299cd6c-cygwin-opam-ocaml.5.1.0

https://github.com/ocaml-multicore/multicoretests/actions/runs/4469170087/jobs/7850901158?pr=311

Is this expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants