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.

Sign up for GitHub

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

Jump to bottom

Build ci dune alias rather than runtest #20

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Build ci dune alias rather than runtest #20

merged 1 commit into from
Jul 7, 2023

Conversation

shym
Copy link
Contributor

@shym shym commented Jul 7, 2023

This PR aligns what is done in both CI systems.

@shym
Build ci dune alias rather than runtest
163b12d 
Use the `ci` dune alias created to configure what tests should really
run in CI, rather than the general `runtest` alias
Run the internal tests (in `test/`) in a first step

Also add the `--error-reporting=twice` option to get a summary of the
tests that failed at the end of the log
jmid
jmid approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@jmid jmid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ocaml-multicore/multicoretests#366 is the corresponding PR changing the GitHub actions runners accordingly.

The primary change is to instead run dune build @ci which lets us run a "focused test" of one particular test file, to understand its behaviour across the many architectures/platforms - and now CI systems!
(commit message has more details)

@benmandrew
Copy link
Collaborator

Is it intended to maintain dune runtest -j1 --no-buffer --display=quiet test/ specifically for the test/ directory alongside the new @ci alias? At first glance ocaml-multicore/multicoretests#366 appears only to execute the @ci alias.

@jmid
Copy link
Contributor

jmid commented Jul 7, 2023

Yes, the intention is to run dune runtest -j1 --no-buffer --display=quiet test/ separately first, outside the new @ci alias 👍

These represent "internal tests of the STM/Lin/... libraries".
We try to separate these from the "multicore torture tests" and also run them separately in a preceding step in the GitHub actions workflow: https://github.com/ocaml-multicore/multicoretests/blob/main/.github/workflows/common.yml#L282

@benmandrew benmandrew merged commit 1cfa78e into ocurrent:main Jul 7, 2023
@shym shym deleted the build-ci-alias branch July 7, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmid jmid jmid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shym @benmandrew @jmid