Running tests together on github-actions.

[prady0t]

Description

Running unit and integration tests together for speedup.

Fixes #4099

[agriyakhetarpal]

I guess this duplicates the coverage job but we can look at this further here itself, could you please fix the conflicts first before I leave another review?

[prady0t]

I guess this duplicates the coverage job but we can look at this further here itself, could you please fix the conflicts first before I leave another review?

Using the exclude block would have resulted in integration tests also getting excluded. Can we also run coverage test together with this?

[agriyakhetarpal]

Using the exclude block would have resulted in integration tests also getting excluded. Can we also run coverage test together with this?

You can do something like

      - name: Run unit/coverage and integration tests
        run: |
          if [ ${{ matrix.python-version }} == "3.12" && ${{ matrix.os }} == "ubuntu-latest" ]; then
            python -m nox -s coverage
          else
            python -m nox -s unit
          fi
          python -m nox -s integration

and then include the excluded Linux 3.12 job. You might need to either add shell: bash to the step or adjust the syntax for pwsh on Windows (former is better)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.54%. Comparing base (6ba81b7) to head (4077c31).

❗ Current head 4077c31 differs from pull request most recent head eb586f0

Please upload reports for the commit eb586f0 to get more accurate results.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4108   +/-   ##
========================================
  Coverage    99.54%   99.54%           
========================================
  Files          287      287           
  Lines        21753    21753           
========================================
  Hits         21655    21655           
  Misses          98       98           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[prady0t]

Using the exclude block would have resulted in integration tests also getting excluded. Can we also run coverage test together with this?

You can do something like

      - name: Run unit/coverage and integration tests
        run: |
          if [ ${{ matrix.python-version }} == "3.12" && ${{ matrix.os }} == "ubuntu-latest" ]; then
            python -m nox -s coverage
          else
            python -m nox -s unit
          fi
          python -m nox -s integration

and then include the excluded Linux 3.12 job. You might need to either add shell: bash to the step or adjust the syntax for pwsh on Windows (former is better)

Wouldn't the above if else condition already do the job? Do we still need to add "exclude block" after this?

[agriyakhetarpal]

Wouldn't the above if else condition already do the job? Do we still need to add "exclude block" after this?

We're referring to the same thing, yes – "include the excluded block" would be the same as not excluding it. We can then move the coverage upload step to this job and run it with an if: condition.

[agriyakhetarpal]

Thanks, @prady0t! Looks all good to me. The Windows integration tests look like they were on a slower runner, and we can't do much about this until we migrate all the integration tests to pytest and thereby make use of pytest-xdist. I'll merge this in a while if there are no objections.

For additional context for any other reviewers: we can receive 12 or so concurrent jobs from GitHub runners on a free plan, so requesting 40 or so jobs means we need to wait until some of them pass in order to run further requested jobs, which meant that we were not benefiting from the parallelisation between unit and integration tests. With this PR, the number of requested jobs is now 24 (including the style and the link checkers).