subject:"\[Django\] #20483\: Reduce the set of apps seen by individual tests"

#20483: Reduce the set of apps seen by individual tests
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Testing framework|   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by aaugustin):

 * status:  new => closed
 * resolution:   => fixed
 * severity:  Release blocker => Normal
 * stage:  Accepted => Ready for checkin


Comment:

 This ticket is too long, I've created #20579 to discuss this problem. Re-
 closing.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.b0b504f0b40ce448db81faa2273d9c8b%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Release blocker   |   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by aaugustin):

 * status:  closed => new
 * resolution:  fixed =>
 * severity:  Normal => Release blocker
 * stage:  Ready for checkin => Accepted


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.6fe4d5479cdc91de685035842d3d9dc5%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by akaariai):

We have a problem... flush is called as part of teardown of TXTestCase. So
flush installs contenttypes / permissions / based on the current test's
available_apps, but the contenttypes/permissions are used by the next
test. This of course isn't correct.

This isn't easy to fix. If we move flush to pre-test, that leads to other
problems (we moved flush to post-test just some time ago). We can't just
stop creating contenttypes & permissions in flush unconditionally. But I
think that is still the way to go: flush the db after test so that it is
totally empty, the next test will then add required data, including
contenttypes & permissions to the test database. But I don't know how to
actually do this. Maybe we could just call "flush but don't create
anything" in post-teardown, then explicitly call "create data" in setup.
But this is a scary change to do in alpha stage...

As the changes in this ticket affect only Django's test suite and
everything works OK there the situation isn't that bad. But this should
still be fixed ASAP.

I noticed the problem while working on some more
[https://github.com/akaariai/django/compare/available_apps_more test
speedups] - some tests use syncdb directly in TestCase subclasses and the
result is creation of a lot of contenttypes & permissions for no reason.
As a positive sidenote: the branch gives almost 20% faster tests with
relatively minor changes, that is 200s -> 165s on sqlite.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.3845616264da1a4a4d6e3d38341100a9%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Testing framework|   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Aymeric Augustin ):

 In [changeset:"dfcce4288ac6887e9995c48f2fde38f555cec03e"]:
 {{{
 #!CommitTicketReference repository=""
 revision="dfcce4288ac6887e9995c48f2fde38f555cec03e"
 Fixed available_apps for selenium tests.

 Refs #20483.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.f127a3c3ca76214eb58646f0f8b48875%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Testing framework|   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Aymeric Augustin ):

 In [changeset:"c6e6d4eeb776c473567362405cdbc6a0328eb194"]:
 {{{
 #!CommitTicketReference repository=""
 revision="c6e6d4eeb776c473567362405cdbc6a0328eb194"
 Defined available_apps in relevant tests.

 Fixed #20483.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.2f964610e64a3644e799a36a8d0ea02a%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"4daf570b98cc840e1a154f3876bc7463924cb9ae"]:
{{{
#!CommitTicketReference repository=""
revision="4daf570b98cc840e1a154f3876bc7463924cb9ae"
Added TransactionTestCase.available_apps.

This can be used to make Django's test suite significantly faster by
reducing the number of models for which content types and permissions
must be created and tables must be flushed in each non-transactional
test.

It's documented for Django contributors and committers but it's branded
as a private API to preserve our freedom to change it in the future.

Most of the credit goes to Anssi. He got the idea and did the research.

Fixed #20483.
}}}

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.bde914ed23782958705a638297b0f365%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Testing framework|   Resolution:
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by aaugustin):

 * stage:  Accepted => Ready for checkin


Comment:

 I've ensured that every `TransactionTestCase` and every
 `LiveServerTestCase` has `available_apps` (possibly set to the empty
 list). I've removed superfluous declarations. I've improved the docs a
 bit.

 Updated PR: https://github.com/django/django/pull/1240

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.686d5b88f25041bcabb7e22c65e13103%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by akaariai):

 The available_apps for plain TestCases should be irrelevant.

 It is hard to say how bad state leaks are. These are usually annoying to
 fix (run single test app and tests pass, run the full test suite and you
 get a failure). I guess we can commit this and then add some autodetection
 of leaked state if need be.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.30da056caeadd1bf4963a50128e190d9%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 A few `TestCase`s (not `TransactionTestCase`s) have `available_apps`
 declared in the current patch. I inherited this from your original proof
 of concept.

 Is it still useful, or is it irrevelant now that fixture loading is
 optimized?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.f250013ed9aeb77393074af4b3e827ab%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 Tests tend to blow up rather quickly and badly when you forget something
 in `available_apps`, but regardless, you are right -- silent leaks could
 occur. Now, if a leak doesn't have any consequences, how bad is it? :)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.79bd719e5638a8d8fdb98bbcf81a36ba%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by akaariai):

I wonder if there are databases where TRUNCATE CASCADE isn't available,
but TRUNCATE alone isn't possible... I guess such databases could always
use DELETE for the tables. So, this seems OK.

I added one comment in the pull request but that isn't anything that can
be acted on in this patch.

The biggest worry is that there seems to be zero safety net for querying &
modifying data outside the available apps. The question is: how do you
know the available_apps you have are actually correct? Tests passing
doesn't mean they are correct, it just means that currently the possible
data leaks between TransactionTestCases do not matter. Also, how do you
know the installed_apps remains correct? (You can create sessions for
example in setUp for transaction tests and the data is leaked between
tests).

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.77752c9b48e049399022c2fab83a4337%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by aaugustin):

 * needs_better_patch:  1 => 0


Comment:

 This patch is about as good as I can make it, it's ready for a final
 review.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.6f4305ec8a585764ba63bffd71fb2e49%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 I updated the pull request again to use `TRUNCATE ... CASCADE` when
 flushing after a `TransactionTestCase` when `available_apps` is set.

 Run time reported by the test runner, with Python 2.7:

 * SQLite: 200s
 * Postgres: 273s
 * MySQL: 230s

 If memory serves, on the same machine, run time used to be 10 minutes on
 SQlite, 20 minutes on PostgreSQL, and about the same on MySQL.

 This is kinda cool.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.b1482055d9a628a3fbadafffd1d76bfd%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

I updated my pull request (https://github.com/django/django/pull/1240) as
follows:

- I simplified the behavior and minimized changes by removing everything
that isn't strictly necessary.
- I updated `available_apps` declarations, since the current application
isnt' implicitly included any longer.
- I refactored the changes to the app cache, `AppCache.available_apps` is
now a set of app labels, and I added `unset_available_apps()` which looks
slightly better than `set_available_apps(None)`.
- I added documentation: the API is still explicitly described as private
and not subject to the deprecation policy, but it's explained for
contributors and committers.

Tests pass under SQLite, I'm stumbling on the same problem as Anssi:
`flush` fails on PostgreSQL. I see two possibilities:
- use `TRUNCATE .. CASCADE` -- since this happens on the test database,
and isn't recommended outside of Django's test suite, I'm not too
concerned about cascading to unmanaged models and destroying data.
- use `DELETE FROM ...` instead of `TRUNCATE ...` -- this needs a
benchmark.

Either solution requires a non-trivial amount of changes :(

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.e08d6bcc0480590ed0957bccc3ba022d%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 Replying to [comment:25 aaugustin]:
 > I thought requiring every related app to be in `available_apps` solved
 this problem?

 I missed comment 21. Sorry for the noise.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.9cdc3ee823e4f63e8789a3093f276b3f%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

Replying to [comment:18 ptone]:
> create a separate instance of an app_cache that is a filtered clone of
the full version, and then set the global django.db.models.loading.cache
global to that filtered version for the testcase, and restore it on
teardown. This way all the filtering logic that is specific to test-
optimization happens in the testing code, and the app_cache instance
doesn't know any different.

That's a good idea, but it isn't going to be enough.
`django.db.models.loading` exposes all methods of the app cache singleton
as module-level functions. You'd have to swap all these too. Doable, but
messy.

Lots of things could be refactored with a cleaner app cache...

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.2fb8d4719a24d03e944a218ecd8c6322%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

Replying to [comment:23 akaariai]:
> We can get rid of the delete cascade problem - any model outside
currently available apps should be empty, so cascading to those models
should not do anything in any case. But the truncate cascade problem is
harder. PostgreSQL won't let you truncate a table if all dependant tables
aren't truncated in the same query, even if those dependant tables are
empty. We can either use TRUNCATE CASCADE (but this can cascade to models
outside of Django's control), or the current logic of expanding cascades.

I thought requiring every related app to be in `available_apps` solved
this problem?

I wanted to document it like this:
https://github.com/aaugustin/django/commit/4717a1e

If the trade-off is "a few more apps in available_apps" vs. "logic for
expanding cascades", I'm choosing the first one :)

As far as I can tell, for Django's own test suite, we don't have the
problem because we don't have inter-app FKs, do we?

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.e9871906cb70ca519d7bd06a7b841578%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

Question: in current patches, the app containing the test is implicitly
included. But with the new test discovery tests can live outside apps. I
understand that including the current app in `available_apps` would be
slightly more verbose, but it's also more consistent and explicit.

The code for implicitly including the current app looks sufficiently
robust, I don't have anything against in from a technical point of view.
I'm really considering this from an API point of view -- ie. terse vs.
explicit.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.ffe93e04691da2e3dc83c8ac43cf50d4%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by akaariai):

While working on the experimental branch for using override_settings, I
stumbled upon this same issue. The end result is weird - if you use
override_settings(INSTALLED_APPS) only apps in INSTALLED_APPS are usable.
If you don't apps outside current INSTALLED_APPS are usable.

The branch contains at least one nice addition, too. It seems evident that
we can (and IMHO should) get rid of the get_model(masked=True/False) and
instead use the "only_installed" flag for this purpose, too. All the
places that need changes to masked kwarg already happen to use
only_installed.

We can get rid of the delete cascade problem - any model outside currently
available apps should be empty, so cascading to those models should not do
anything in any case. But the truncate cascade problem is harder.
PostgreSQL won't let you truncate a table if all dependant tables aren't
truncated in the same query, even if those dependant tables are empty. We
can either use TRUNCATE CASCADE (but this can cascade to models outside of
Django's control), or the current logic of expanding cascades.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.3431ccfc827ef15eb06237428ad16453%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

With the current implementation of the app cache, it can contain models
from an application that isn't in `INSTALLED_APPS`. All you need is to
import the model, and the metaclass `ModelBase` will register the model
with `register_models`. It automatically determines the app, see around
the comment `# Figure out the app_label by looking one level up.` in
`django/db/models/base.py`.

So `override_settings(INSTALLED_APPS=just_a_few_apps)` sounds like a poor
API to removing models from the app-cache. Even if `INSTALLED_APPS` was
set to `just_a_few_apps` in the first place, other models could still be
added to the cache if they're imported somehow.

(In the long term, I think the app cache should prevent loading models
when their app isn't in `INSTALLED_APPS`. Maybe app-loading does this, I'm
not sure.)

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.10da5d29542fbf3d695b60e23668595c%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

2013-06-06 Thread Django

Comment (by akaariai):

I did some more hacking. This time I tried to use
override_settings(INSTALLED_APPS). The result is maybe a bit cleaner, but
then again, maybe not...

It is now evident that we can't get fully rid of global state. The problem
is table truncation and the need to cascade into applications not in
current available_apps set. For example you might have app1 and app2 that
don't know about each other, but both contain foreign key to auth.user.
Now when app1 does flush, it will flush also auth.user, and that flush
needs to cascade to app2. This is impossible to do unless we want to use
TRUNCATE CASCADE (which could cascade to unmanaged models etc), or we have
global installed apps and models available somewhere (likely in global
app-cache). So, end result doesn't look much cleaner. (Fundamental problem
here is that the database schema itself is global state that is extremely
hard to get rid of).

Patch at https://github.com/akaariai/django/tree/pull_1240. We might still
try how things look when instantiating multiple app-caches. But I am not
sure if that will result in anything truly cleaner.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.50f8411f4a7868d7d86eb7d5fe171b24%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

2013-06-06 Thread Django

Comment (by akaariai):

I think schema migrations will need a way to switch app-cache anyways.
Schema migrations will need to use different set of models, not just
"mask" some of them. So changing the app-cache is something we will likely
need in any case.

Swapping the app-cache isn't zero-risk business. Currently the app-cache
class is borg pattern (meaning every instance shares state). This must be
first changed so that we can create instances of the app-cache with
different state. Then we will also need app-cache swapping logic. To me
this is much higher risk than having some filtering capability in the app-
cache. In the current patch it is very likely that the app-cache itself
will not break if no "app mask" is set. If you remove the borg pattern
from app-cache who knows what might break in user apps (likely nothing,
but this is much harder to see)?

Another issue is the changes all around Django's code base. Those aren't
zero risk. I think we can get rid of almost all of those.

I tried the available_apps = self + ALWAYS_INSTALLED_APPS for every
TransactionTestCase, results for PostgreSQL and transactions +
transactions_regress is 54s, with Aymeric's patch from pull 1240 it is
5.6s, and on master it is 60s. Full test suite on master is around 4500s,
around 750 with self + ALWAYS_INSTALLED_APPS and 300s on pull 1240. In
addition there is one test that needs more than its own app
(proxy_model_inheritance.tests.ProxyModelInheritanceTests.test_table_exists()).
So, ALWAYS_INSTALLED_APPS is possible, but it doesn't give as good
performance as setting available_apps to smallest possible set and most of
all it doesn't solve any of the hard problems (app-cache changes or
changes in non-test code).

I will next try doing the following changes:
- all dependent apps need to be in the available_apps (that is, delete &
truncate cascades, too). This will get rid of the backends changes, and
changes in deletion.
- use of override_settings(INSTALLED_APPS=available_apps). This way we
should be able to get rid of changes in the signal listeners for
permissions, contenttypes and super user creation. Instead we can
disconnect the signal itself.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.e42c39266f2f6eacd435425bd0c591fa%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

2013-06-05 Thread Django

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 Well, I think it's worth exploring a few alternative implementations
 before committing this optimization. Your proposal is interesting.

 My main goal is to avoid as much as possible touching non-test code -- ie.
 anything outside of django/test/ and tests/.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.f924db5f5f9124a54102a2f4c1a4f21a%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

2013-06-05 Thread Django

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by ptone):

 I've given this a look over and had only a couple minor comments - I'm of
 course very excited for the speedup!

 My only major comment is I'd love for this to be possible without touching
 the app_cache logic, it seems less than ideal to have a bunch of test
 optimization code inside the app_cache. I don't think that is any reason
 alone not to land this improvement. I would at least add some comments as
 to the purpose of the available/masking business in the docstring, or some
 comments to get_model*

 Here is an idea of how this could be implemented differently, without
 changing the proposed API from the testcase point of view:

 create a separate instance of an app_cache that is a filtered clone of the
 full version, and then set the global django.db.models.loading.cache
 global to that filtered version for the testcase, and restore it on
 teardown. This way all the filtering logic that is specific to test-
 optimization happens in the testing code, and the app_cache instance
 doesn't know any different.

 While I think that is a bit "better" I really don't feel  that it is worth
 redoing this work given this is working and nearly ready - and it could
 always be a task that gets done later since we are talking about just some
 internals of Django's internal test running - I do agree with comments
 above that this should be private API for now.

 It does seem that we should document this in:

 https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
 /unit-tests/

 as a practice to follow for Django's own tests.

 But overall the benefits are far outweighing any of the cons I've brought
 up.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.9063df96b7affed7e2fe5344fdfb6d1c%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 I've started working on a branch:
 https://github.com/django/django/pull/1240. I will force-push revisions to
 that branch until it's ready.

 I have one significant concern at this point: this idea requires quite a
 few changes in code that isn't related to tests. I'll look into minimizing
 these changes.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.970372fb51bf58be51c75babc3109398%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+
Changes (by aaugustin):

 * needs_docs:  1 => 0


Comment:

 Yes, we can keep it private for the time being. Anyway it's quite specific
 to Django's own test suite. I suspect it's fairly uncommon to have
 hundreds of *unrelated* models.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.b54200d4393b8202e6e0044039cec797%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  1
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by charettes):

 Alright since this alternate mechanism is introduced because of the
 limitation of the app_cache may I suggest we avoid documenting it until
 #3591 is fixed and we can revisit the approach taken here?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.d8fcd91c59076404c91288920c5c7402%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by aaugustin):

The main problem with the app cache is that it's populated at compile
time. This task is performed by the ModelBase metaclass.

This has a few unfortunate consequences:
- importing an application's models module makes its models available in
the app cache regardless of INSTALLED_APPS;
- it's very hard to control (eg. save and restore) the state of the app
cache in a given Python process; one can't re-run the import process as
Python will just look up the module in `sys.modules`.

Once in a while someone tries to create models dynamically in a test, and
it never works well :)

Since it's impossible to emulate cleanly a different value of
INSTALLED_APPS, at least as far as the app cache is concerned, I prefer
using another mechanism.

The problem we're trying to work around here is "TransactionTestCase is
slow as molasses because it makes 4000 inserts for each test". It makes
sense (to me) to solve this at the level of TransactionTestCase.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.f47a42c8bb605f3fc52fa675d2bf6e05%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

Comment (by akaariai):

The available_apps name sounds good to me.

Currently truncation on PostgreSQL will need to cascade to apps outside
available_apps, this is an example where override_settings !=
available_apps. But now that I think of this the proper fix to this issue
isn't automatic cascade outside available_apps - why not include the
cascades in available_apps, too? Seems like there is a dependency from the
test case's models to the apps we need to cascade, so marking this
dependency explicitly seems like the right thing to do. If this leads to
crazy amounts of cascades maybe we need to think about why the cascades
are needed in the first place.

It isn't actually that easy to find good reasons why override_settings
must be different than available_apps. One issue that comes to mind is
that available_apps needs to be subset of INSTALLED_APPS. I don't think
that needs to be true for override_settings. Another problem is that at
least currently we can't actually flush the whole app-cache, so there
might be issues where this isn't working like changed INSTALLED_APPS.

Still, I think that marking app-dependencies is special enough to warrant
nice declarative API. Also, we might want to mark available_apps for every
TestCase in Django so that we can get rid of runtests.py
ALWAYS_AVAILABLE_APPS. Check all dependencies of the tests we are going to
run. That is our set of INSTALLED_APPS, and no need to include
ALWAYS_AVAILABLE_APPS in there. This seems to be a lot easier to achieve
with available_apps than override_settings based implementation.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.287d89d783627d163cabe49971cca0f6%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  1
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by charettes):

 @akaariai what's the technical reason that prevent us from reusing the
 `override_settings(INSTALLED_APPS=[...])` mechanism instead of introducing
 a new one?

 Can't `TransactionTestCase` register a signal receiver and just call
 `cache.set_app_mask`?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.2d9c201aa416abff33d0176d1aaf9ba6%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  1
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by mjtamlyn):

 +1 to `available_apps` - I'd been trying to think of a good name but I
 think that's about right.

 `mask` was totally confusing terminology to me to start with...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.149cedea0b2dc6eadda6d91cec09cb7f%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests

#20483: Reduce the set of apps seen by individual tests
--+
 Reporter:  akaariai  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Testing framework |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  1
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 For `app_mask`: what about `available_apps`?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.61d84156abc95b2e41da40b196b7e939%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #20483: Reduce the set of apps seen by individual tests