Category: Code

When possible, I very much recommend adding unit tests to a Pull Request when you fix something as a way to prevent that it breaks again in future. This is no news.

One important step of adding that unit test, though, is to make sure it actually tests the bug you are fixing. Specifically, this means that you need to test that fact like this:

Keep the new unit test, undo the other code changes. The unit test now needs to fail.

If your newly introduced unit test still passes, then you didn’t expose the bug in the test. A simple check but often neglected.

A tooling solution

Here is how you can automatically test this in Github. I have created a sample repo with 3 pull requests https://github.com/akirk/unit-test-failure/pulls:

You can see the first one failed:

The problem is that the old code already passes the new tests. This means the tests don’t test the change.

The second one passes but only because no new tests were added at all.

And the third one does it correctly. The old code fails the new tests as desired:

Here is the code for the Github Action to test this:

name: Pull Request Unit Test Validation

on:
  pull_request:
    types:
      - opened
      - synchronize

jobs:
  test-changes:
    runs-on: ubuntu-latest

    steps:
      - name: Checkout code
        uses: actions/checkout@v2

      - name: Set up Python
        uses: actions/setup-python@v2
        with:
          python-version: 3.9

      - name: Run unit tests with changes
        run: python -m unittest discover -s tests

  test-without-changes:
    needs: test-changes
    runs-on: ubuntu-latest
    if: ${{ github.event_name == 'pull_request' }}

    steps:
      - name: PR commits + 1
        run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}"

      - name: Checkout PR branch and all PR commits
        uses: actions/checkout@v3
        with:
          ref: ${{ github.event.pull_request.head.sha }}
          fetch-depth: ${{ env.PR_FETCH_DEPTH }}

      - name: Set up Python
        uses: actions/setup-python@v2
        with:
          python-version: 3.9

      - name: 'Fetch the other branch with enough history for a common merge-base commit'
        run: git fetch origin ${{ github.event.pull_request.base.ref }}

      - name: Check if tests were changed
        run: |
          git restore --source=$(git merge-base origin/${{ github.event.pull_request.base.ref }} HEAD) --worktree tests/
          if git diff --quiet; then
            echo "TESTS_CHANGED=0" >> "${GITHUB_ENV}"
          else
            echo "TESTS_CHANGED=1" >> "${GITHUB_ENV}"
          fi
          git restore .

      - name: Revert code changes (excluding tests)
        run: git restore --source=$(git merge-base origin/${{ github.event.pull_request.base.ref }} HEAD) --worktree :^tests/
        if: ${{ env.TESTS_CHANGED }} == '1'

      - name: Run unit tests without changes
        run: |
          if [[ ${{ env.TESTS_CHANGED }} -eq 0 ]]; then
            echo "No unit test changes detected, skipping"
            exit 0
          fi

          if python -m unittest discover -s tests; then
            echo "Unit test should not pass"
            exit 1
          else
            echo "Unit test failed as expected"
          fi

Thanks @jesusamieiro-com for pointing out that you need to be careful with this for code that adds new test coverage. Probably best to use a Github label to activate or deactivate this action.