haskell / actions

Github actions for Haskell CI
147 stars 54 forks source link

Changes for issue 215 #230

Closed sgadkar2 closed 1 year ago

sgadkar2 commented 1 year ago

Fixes #215.

andreasabel commented 1 year ago

I notice we do not test stack installation much, so CI does not cover this change. https://github.com/haskell/actions/blob/f334d58da81a3d707ada2ce33c610252737af896/.github/workflows/workflow.yml#L201-L218 Can you think of a way to test in CI that the returned stack-root output is correct?

sgadkar2 commented 1 year ago
install-stack:
    name: Stack ${{ matrix.stack }} ${{ matrix.os }}
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os: [ubuntu-latest, macOS-latest, windows-latest]
        stack: ["latest", "2.3.3"]

    steps:
      - uses: actions/checkout@v3
      - uses: ./setup
        with:
          enable-stack: true
          stack-no-global: true
          stack-version: ${{ matrix.stack }}
      - run: |
          stack --version
          stack --help

      - name: Checking if returned stack-root output is correct
        shell: bash
        run: |
          actual_stack_root=$(stack path --stack-root)
          if [ "$actual_stack_root" != "${{ steps.setup.outputs.stack-root }}" ]; then
            echo "stack-root does not match expected value"
            exit 1
          fi

@andreasabel will this be enough ?

andreasabel commented 1 year ago

Yes! This looks good for a start. It certainly covers the test for this PR, which is all I want to ask for here.

echo "stack-root does not match expected value"

It could be helpful to include expected and actual value in this print-out, for better error comprehension and debugging.

andreasabel commented 1 year ago

@Mergifyio rebase

mergify[bot] commented 1 year ago

rebase

✅ Branch has been successfully rebased

andreasabel commented 1 year ago

After experiences with #231 I am now a bit doubtful about my idea to use stack path to compute stack-root. This call can be expensive (e.g. under Windows where MSYS2 is installed). If one isn't careful, this even installs GHC: https://github.com/haskell/actions/actions/runs/4623372581/jobs/8177114700#step:5:31

Maybe it is better to leave things as they are and be confined with having a test now that ensures the correctness (#231).

sgadkar2 commented 1 year ago

Okay sure. Thanks, it was a good learning experience

andreasabel commented 1 year ago

Thanks again for conducting this experiment!