search

jazzband / django-oauth-toolkit

OAuth2 goodies for the Djangonauts!
https://django-oauth-toolkit.readthedocs.io
Other
3.06k stars 777 forks source link

Speed up tests ~2x #1352

Closed adamchainz closed 7 months ago

adamchainz commented 8 months ago

Description of the Change

Speed up the test suite to be about twice as fast, using these techniques:

  1. Move model creation into setUpTestData(), so it runs only once per test case. See my post.
  2. Remove tearDown() methods that removed model instances. These were always unnecessary as Django resets the database for you.
  3. Move immutable RequestFactorys to class-level variables.

Measuring on the Python 3.12 and Django 5.0 environment (added in #1350), I got 207s before these changes and 105s afterwards, approximately twice as fast.

Checklist

codecov[bot] commented 8 months ago

Codecov Report

Merging #1352 (8227484) into master (fa4bcdf) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1352   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files          32       32           
  Lines        2120     2120           
=======================================
  Hits         2068     2068           
  Misses         52       52

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

adamchainz commented 8 months ago

setUpTestData copying was only added in Django 3.2, before that the djagno-testdata package is needed. I think we should merge #1350 first to drop Django 2.2 support, then I'll rebase this.

n2ygk commented 6 months ago

@adamchainz @dopry not sure why but I've noticed that all the tests run and then the last two success jobs seem to hang for a long time waiting for a runner. Maybe because success is a different job from build? Is there a way to add the Success step into the build job somehow? I'm not sure how to skip the matrix part within the build job...

dopry commented 6 months ago

@n2ygk, Success needs to wait on the matrix build to complete. It can't be bundled into the same. can you open a new issue linking to an example?

n2ygk commented 6 months ago

@dopry not point opening a new issue if we are stuck with the current situation. Still better than the old way.

dopry commented 6 months ago

If you can't point to an example of the issue I can't troubleshoot it further and I suspect it is not directly related to reducing the overheat to setup test data, but is more likely related to capacity or organizational throttling by GH. Correlation is not causation.

n2ygk commented 6 months ago

@dopry https://github.com/jazzband/django-oauth-toolkit/issues/1376

n2ygk commented 6 months ago

But I think I should have been commenting on #1219