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

CI accidentally skips over crux-llvm tests on macOS #999

Closed
RyanGlScott opened this issue Jun 14, 2022 · 0 comments · Fixed by #1172
Closed

CI accidentally skips over crux-llvm tests on macOS #999

RyanGlScott opened this issue Jun 14, 2022 · 0 comments · Fixed by #1172
Assignees
Labels
CI crux llvm macOS Issues specific to macOS
Milestone

Comments

@RyanGlScott
Copy link
Contributor

RyanGlScott commented Jun 14, 2022

Recently, I came to wonder why the macOS CI jobs weren't catching #885, which should prevent the crux-llvm tests from passing on macOS. It turns out this is because the CI isn't actually running the crux-llvm test suite on macOS at all! Take a look at what happens on a typical macOS job like this one:

+ /Users/runner/work/crucible/crucible/llvm/bin/clang-withIncl --version
clang-11: error: no input files
+ echo clang version unknown
clang version unknown
+ /Users/runner/work/crucible/crucible/llvm/bin/llvm-link --version
LLVM (http://llvm.org/):
+ cabal v2-test crux-llvm
  LLVM version 11.0.0
  Optimized build.
  Default target: x86_64-apple-darwin19.6.0
  Host CPU: ivybridge
Build profile: -w ghc-9.2.2 -O1
In order, the following will be built (use -v for more details):
 - crux-llvm-0.6.0.99 (test:crux-llvm-test) (first run)
<snip>
Running 1 test suites...
Test suite crux-llvm-test: RUNNING...
Test suite crux-llvm-test: PASS
Test suite logged to:
/Users/runner/work/crucible/crucible/dist-newstyle/build/x86_64-osx/ghc-9.2.2/crux-llvm-0.6.0.99/t/crux-llvm-test/test/crux-llvm-0.6.0.99-crux-llvm-test.log
1 of 1 test suites (1 of 1 test cases) passed.

The first suspicious thing here is that the whole test suite takes 12 seconds to run, which is far too short. The second suspicious thing is that clang-withIncl --version prints clang-11: error: no input files instead of the version number. This is a consequence of how the CI is setting up clang on macOS. It assembles a wrapper script named clang-withIncl here:

echo "#!/usr/bin/env bash" > llvm/bin/clang-withIncl
echo "clang -I${{ github.workspace }}/llvm/include -I${{ github.workspace }}/llvm/include/c++/v1" >> llvm/bin/clang-withIncl

There's a mistake here, however: the invocation of clang doesn't pass through the arguments (e.g., with ${1+"$@"}), so no matter what arguments you pass to the clang-withIncl script, it will simply invoke clang with no input files. We should change this script so that it plumbs through the arguments.

@RyanGlScott RyanGlScott added this to the Crux 0.7 milestone Jun 14, 2022
@kquick kquick self-assigned this Jun 14, 2022
@RyanGlScott RyanGlScott added the macOS Issues specific to macOS label Jul 20, 2022
RyanGlScott added a commit that referenced this issue Jul 21, 2022
Previously, the `clang-withIncl` script that we concocted on CI for calling
`clang` with the correct flags neglected to actually pass the _arguments_ to
`clang`. As a result, the `crux-llvm` test suite would trivially pass on macOS
since it skipped all of the test cases due to `clang-withIncl` being
misconfigured. This corrects this mistake by passing the arguments through to
`clang-withIncl` using `${1+"$@"}`.

Fixes #999. This won't be enough on its own to make the `crux-llvm` test suite
pass on macOS—see #885 and #1013. Fixes for those issues will come in
subsequent commits.
RyanGlScott added a commit that referenced this issue Jul 21, 2022
Previously, the `clang-withIncl` script that we concocted on CI for calling
`clang` with the correct flags neglected to actually pass the _arguments_ to
`clang`. As a result, the `crux-llvm` test suite would trivially pass on macOS
since it skipped all of the test cases due to `clang-withIncl` being
misconfigured. This corrects this mistake by passing the arguments through to
`clang-withIncl` using `${1+"$@"}`.

Fixes #999. This won't be enough on its own to make the `crux-llvm` test suite
pass on macOS—see #885 and #1013. Fixes for those issues will come in
subsequent commits.
RyanGlScott added a commit that referenced this issue Jul 22, 2022
Previously, the `clang-withIncl` script that we concocted on CI for calling
`clang` with the correct flags neglected to actually pass the _arguments_ to
`clang`. As a result, the `crux-llvm` test suite would trivially pass on macOS
since it skipped all of the test cases due to `clang-withIncl` being
misconfigured. This corrects this mistake by passing the arguments through to
`clang-withIncl` using `${1+"$@"}`.

Fixes #999. This won't be enough on its own to make the `crux-llvm` test suite
pass on macOS—see #885 and #1013. Fixes for those issues will come in
subsequent commits.
RyanGlScott added a commit that referenced this issue Apr 20, 2023
The tests were previously skipped on macOS due to a fluke (see
#999), but with the changes made to
the crux-llvm test suite, this no longer occurs. Let's explicitly disable the
tests on macOS until #885 is fixed.
RyanGlScott added a commit that referenced this issue Feb 13, 2024
Previous, we concocted a `clang-withIncl` script for running Clang on macOS CI
runners, but this neglected to pass any arguments to Clang (among other
issues). This patch replaces the `clang-withIncl` machinery with a much
simpler, Homebrew-based setup for install Clang/LLVM. With this in place, we
can finally run the `crux-llvm` test suite on macOS.

Fixes #999.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI crux llvm macOS Issues specific to macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants