Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette ):

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


Comment:

 In [changeset:"a08fda2111d811aa53f11218fa03f3300dfff4cb" a08fda21]:
 {{{
 #!CommitTicketReference repository=""
 revision="a08fda2111d811aa53f11218fa03f3300dfff4cb"
 Fixed #25746 -- Isolated inlined test models registration.

 Thanks to Tim for the 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/067.958d0849874aaa7b3b8e6f04afe8f139%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette ):

 In [changeset:"7bb373e3097fe8e000e0bba005ff2dcfc18ab9a5" 7bb373e3]:
 {{{
 #!CommitTicketReference repository=""
 revision="7bb373e3097fe8e000e0bba005ff2dcfc18ab9a5"
 Refs #25746 -- Added a test utility to isolate inlined model registration.

 Thanks to Tim for the 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/067.622d042a2e49ca4be5175971afbe4ea9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by timgraham):

 * stage:  Accepted => Ready for checkin


--
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/067.d3bc019e212b56c919c56a8b39fdb518%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
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 charettes):

 * needs_docs:  1 => 0


Comment:

 Added docs and rebased on top of master.

--
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/067.c5a7fad2ed2e0e24d67e24f7a62f4946%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  1
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by timgraham):

 * needs_docs:  0 => 1


--
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/067.fe3f6b2e7b37ab404389d202e69f086e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
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 charettes):

 * has_patch:  0 => 1


Comment:

 [https://github.com/django/django/pull/5687 PR]

--
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/067.9e8b0ce2ebbeef466076e8acbc4838e4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by timgraham):

 * stage:  Unreviewed => Accepted


Old description:

> Many test in Django's own test suite inline model definition in order to
> prevent table creation or simply isolate them.
>
> Unfortunately most of these model don't explicitly declare an explicit
> `apps` and end up being registered to the global `apps` instance. This
> has three downsides:
>
> 1. The call to `register_model` busts the `apps` and registered models
> (1k+) `_meta` cache. I didn't run any benchmark here but I suspect this
> slow down things a bit.
> 2. If a model is already registered for the specified
> `app_label.ModelName` (see the `schema` issue detailed in #25745) a
> `RuntimeWarning` is raised.
> 3. If no model is registered for this particular `app_label.ModelName`
> any future call to `migrate` will end up creating the table since we
> don't have explicit migration defined for the test apps.
>
> To prevent the enumerated issues the inlined models should use a per-test
> isolated registry. Examples of such a pattern can be found on this
> [https://github.com/django/django/pull/5613 PR].
>
> e.g.
>
> {{{#!python
> def test_foo(self):
> test_apps = Apps(['test_app'])
> class Foo(models.Model):
> class Meta:
> apps = test_apps
> }}}
>
> In order to reduce the boiler plate required to achieve isolation I
> suggest introducing a test decorator/context manager that register models
> declared in it's body in an isolated `Apps()` instance.
>
> e.g.
>
> {{{#!python
> @isolate_apps('test_app', 'test_app_2')
> def test_foo(self):
> class Foo(models.Model):
> pass
>
> with isolate_apps('test_app3') as apps:
> class Bar(models.Model):
> class Meta:
> app_label = 'test_app3'
> assert Bar is apps.get_model('test_app3', 'Bar')
> }}}
>
> This would also make fixing the actual and future isolation problems
> quite easy by simply decorating the faulty test.

New description:

 Many tests in Django's own test suite inline model definition in order to
 prevent table creation or simply isolate them.

 Unfortunately most of these model don't explicitly declare an explicit
 `apps` and end up being registered to the global `apps` instance. This has
 three downsides:

 1. The call to `register_model` busts the `apps` and registered models
 (1k+) `_meta` cache. I didn't run any benchmark here but I suspect this
 slow down things a bit.
 2. If a model is already registered for the specified
 `app_label.ModelName` (see the `schema` issue detailed in #25745) a
 `RuntimeWarning` is raised.
 3. If no model is registered for this particular `app_label.ModelName` any
 future call to `migrate` will end up creating the table since we don't
 have explicit migration defined for the test apps.

 To prevent the enumerated issues the inlined models should use a per-test
 isolated registry. Examples of such a pattern can be found on this
 [https://github.com/django/django/pull/5613 PR].

 e.g.

 {{{#!python
 def test_foo(self):
 test_apps = Apps(['test_app'])
 class Foo(models.Model):
 class Meta:
 apps = test_apps
 }}}

 In order to reduce the boiler plate required to achieve isolation I
 suggest introducing a test decorator/context manager that register models
 declared in it's body in an isolated `Apps()` instance.

 e.g.

 {{{#!python
 @isolate_apps('test_app', 'test_app_2')
 def test_foo(self):
 class Foo(models.Model):
 pass

 with isolate_apps('test_app3') as apps:
 class Bar(models.Model):
 class Meta:
 app_label = 'test_app3'
 assert Bar is apps.get_model('test_app3', 'Bar')
 }}}

 This would also make fixing the actual and future isolation problems quite
 easy by simply decorating the faulty test.

--

--
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 

Re: [Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
-+-
 Reporter:  charettes|Owner:  charettes
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Testing framework|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by charettes):

 * owner:  nobody => charettes
 * status:  new => assigned


--
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/067.b51c4e63e1441fcad8a3f91f3e658d2b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #25746: Isolate inlined test models

[Django]

#25746: Isolate inlined test models
+
   Reporter:  charettes |  Owner:  nobody
   Type:  Cleanup/optimization  | Status:  new
  Component:  Testing framework |Version:  master
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 Many test in Django's own test suite inline model definition in order to
 prevent table creation or simply isolate them.

 Unfortunately most of these model don't explicitly declare an explicit
 `apps` and end up being registered to the global `apps` instance. This has
 three downsides:

 1. The call to `register_model` busts the `apps` and registered models
 (1k+) `_meta` cache. I didn't run any benchmark here but I suspect this
 slow down things a bit.
 2. If a model is already registered for the specified
 `app_label.ModelName` (see the `schema` issue detailed in #25745) a
 `RuntimeWarning` is raised.
 3. If no model is registered for this particular `app_label.ModelName` any
 future call to `migrate` will end up creating the table since we don't
 have explicit migration defined for the test apps.

 To prevent the enumerated issues the inlined models should use a per-test
 isolated registry. Examples of such a pattern can be found on this
 [https://github.com/django/django/pull/5613 PR].

 e.g.

 {{{#!python
 def test_foo(self):
 test_apps = Apps(['test_app'])
 class Foo(models.Model):
 class Meta:
 apps = test_apps
 }}}

 In order to reduce the boiler plate required to achieve isolation I
 suggest introducing a test decorator/context manager that register models
 declared in it's body in an isolated `Apps()` instance.

 e.g.

 {{{#!python
 @isolate_apps('test_app', 'test_app_2')
 def test_foo(self):
 class Foo(models.Model):
 pass

 with isolate_apps('test_app3') as apps:
 class Bar(models.Model):
 class Meta:
 app_label = 'test_app3'
 assert Bar is apps.get_model('test_app3', 'Bar')
 }}}

 This would also make fixing the actual and future isolation problems quite
 easy by simply decorating the faulty test.

--
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/052.92830947c9394315c54a5f83e925feb0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.