Suggestion: ci.yml - Split "Test" step into smaller steps

[kennyhei]

Currently in ci.yml, the "Test" step does quite a bit of stuff (in ci:test) before actually running tests:

• Linting and formatting
• Builds Docker image
• Lints and formats again
• Runs migrations
• Runs tests

GitHub Action's log output is filled with info that's not really relevant if you just want to check what's going on with actual test runs. It would be nice if this was split into smaller steps such as:

• Lint & format
• Build app (docker compose build + migrations)
• Test (manage.py test)

[nickjj]

That could be done, although linting and formatting might need to be broken up into multiple steps because you can lint the dockerfile and shell scripts before building anything but to lint the main code base requires building the image first since ruff is installed at that point.

I suppose our options are:

• Split the lint / format steps up
• Lint / format everything after building

Both options have their pros and cons.

Option 1 feels a little convoluted and option 2 could be inefficient if a lint step fails on the Dockerfile.

Between both I probably lean more towards option 2 just to keep linting and formatting together but you could make a case that maybe all 3 things (dockerfile, shell and app) could be their own individual steps too. That seems ok too.

[kennyhei]

Yeah, I've also seen setups where lint & format would be their own job instead of being a step. It looks clean and can be run in parallel with test job although as a downside both jobs would require building image and installing deps (I think)? If duplicate setup steps bother, that is. I wonder if building docker image could be omitted completely for e.g. linting and formatting at least, just install dependencies and let ruff do its magic "locally", not on the container?

[nickjj]

Yep, in parallel would require both to build images. Thankfully linting and formatting takes literally less than 1 second. Even on a bigger project ruff is super fast.

If we went with a local approach it would ensure ruff in CI always matches the version in pyproject.toml. If you wanted to get crazy you could parse out the version programmatically but this feels too complicated. Django and Flask are lucky too because ruff is so easy to install. I have example apps for Rails, Node and Phoenix too, this local approach would be a bit more complicated there.

[kennyhei]

For my project I went with the simplest option which is the following:

      - name: "Build app"
        run: |
          # Remove volumes in CI to avoid permission errors due to UID / GID.
          sed -i "s|.:/app|/tmp:/tmp|g" .env*
          sed -i "s|.:/app|/tmp:/tmp|g" docker-compose.yaml

          # Django requires static files to be collected in order to run its
          # test suite. That means we need to generate production assets from
          # esbuild. This line ensures NODE_ENV is set to production.
          sed -i "s|export NODE_ENV|#export NODE_ENV|g" .env*

          ./run ci:build

      - name: "Test"
        run: |
          ./run manage test

ci:build is the same as ci:test except the last line manage test has been removed

[nickjj]

Thanks, now that I see the output, I do like the idea of splitting things to some degree because it's easier to see how long each step took which is nice to know to watch for regressions or wanting to know how long each part took.