pnpm / action-setup

Install pnpm package manager
https://github.com/marketplace/actions/setup-pnpm
MIT License
904 stars 87 forks source link

cache should be written immediately after install #16

Closed wmertens closed 3 years ago

wmertens commented 3 years ago

I used the example for caching, but if my tests fail, the step for storing the pnpm store in cache fails. Should it not run immediately after installing?

KSXGitHub commented 3 years ago

but if my tests fail, the step for storing the pnpm store in cache fails

That's just how GitHub Workflow works.

wmertens commented 3 years ago

I worked around it by making a separate job for it:

jobs:
  # this needs to be a separate job because failures will fail to cache the pnpm store
  cache:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        node-version: [12.x]
    steps:
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v2
        with:
          node-version: ${{ matrix.node-version }}

      - name: checkout for cache
        uses: actions/checkout@v2

      - name: Cache node_modules
        id: modules
        uses: actions/cache@v2
        with:
          path: node_modules
          key: ${{ runner.os }}-modules-${{ matrix.node-version }}-${{ hashFiles('**/pnpm-lock.yaml') }}

      - name: Get Date
        if: ${{ steps.modules.outputs.cache-hit != 'true' }}
        id: get-date
        run: |
          echo "::set-output name=date::$(/bin/date -u "+%Y%m%d")"
        shell: bash

      - name: Cache pnpm store
        if: ${{ steps.modules.outputs.cache-hit != 'true' }}
        uses: actions/cache@v2
        with:
          path: ~/.pnpm-store
          # this key is used for all repos, and doesn't update after hit, so only use it for a day
          key: ${{ runner.os }}-pnpm-${{ matrix.node-version }}-${{ steps.get-date.outputs.date }}

      - name: pnpm install
        if: ${{ steps.modules.outputs.cache-hit != 'true' }}
        uses: pnpm/action-setup@v2.0.1
        with:
          version: 6.9.1
          run_install: true

  build:
    needs: cache
    runs-on: ubuntu-latest
    strategy:
      matrix:
        node-version: [12.x]
    steps:
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v2
        with:
          node-version: ${{ matrix.node-version }}

      - name: Checkout
        uses: actions/checkout@v2

        # here we use the cache we built in the previous job
      - name: node_modules cache
        uses: actions/cache@v2
        with:
          path: node_modules
          key: ${{ runner.os }}-modules-${{ matrix.node-version }}-${{ hashFiles('**/pnpm-lock.yaml') }}
KSXGitHub commented 3 years ago

That's one way to work around this. But to be honest, I don't think cache is important enough to complicate the code, because:

  1. Failing tests aren't frequent.
  2. Even if failing tests are frequent, just one successful test is enough to initialize cache for subsequent runs.
wmertens commented 3 years ago

:thinking: true... In my case though tests are failing constantly :sweat_smile:

I now was able to limit eslint and jest to only changes on the PR, that solves that.