[Framework-Team] PLIP 20144

Timo Stollenwerk tisto at plone.org
Thu Dec 11 10:51:52 UTC 2014


Hi,

I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.

Cheers,
Timo

Am 09.12.2014 um 13:39 schrieb Johannes Raggam:
> On Tue, 2014-12-09 at 12:10 +0100, Timo Stollenwerk wrote:
>> Am 09.12.2014 um 11:16 schrieb Philip Bauer:
>>> 1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us. 
>>
>> Thanks for bringing this up! :)
> 
> yep.
> 
>> The initial idea was to move all tests to use Dexterity types or the
>> p.a.contenttypes fixture (if necessary). I don't think a generic test
>> fixture that supports both AT and DX is possible.
>>
>> Tests with tons of if/else statements for DX/AT are a horrible idea in
>> my optinion. They are a nightmare to maintain, to debug and really hard
>> to understand.
> 
> In plone.app.event until 1.2 there is a generic test suite for
> Archetypes and Dexterity. I think, this approach was quite elegant:
> 
> In
> https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32 there is an ``AbstractSampleDataEvents``, which creates a basic set of content to test with. The abstract ``event_factory`` method was overridden by AT or DX based unit test classes, like here:
> 
> https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
> 
> You see, ``TestEventViewDX`` defines all tests cases and the concrete
> implementation of ``event_factory`` and uses an DX test layer.
> ``TestEventViewAT`` below just subclasses the DX one, overrides
> ``event_factory`` and uses an AT layer:
> https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
> 
> So both unit tests classes run the same the test cases.
> 
> That's possible, because there are AT and DX content type
> accessors/wrappers, which unify ``set`` and ``get`` access to those
> types.
> 
> Initially this was a proof of concept on how to support both Archetypes
> and Dexterity in one package without duplicating code. It works quite
> well! But we were removing the Archetypes based code from
> plone.app.event 2.0, since this release is Dexterity only.
> 
> To the question on what to do with our Plone tests: Instead of creating
> accessor methods for AT and DX content types, I think it's better to
> rewrite for Dexterity only and move the Archetypes test classes into the
> ATContentTypes package before.
> 
> Johannes
> 
> 
>> Therefore, even though it is not perfect, I think we should stick with
>> the approach to test against Dexterity only. For some packages it might
>> be necessary to actually test both, but those should be the exception
>> from the rule. Most packages just need "some" content object and it does
>> not really matter if it is a DX or AT object.
>>
>> One problem I see with that approach is that the p.a.contenttypes
>> fixture, that most packages use, might be a bit too heavy. If
>> p.a.contenttypes becomes the de-factor test layer we have to make sure
>> that we keep it lightweight (because it is incredibly hard to change a
>> test fixture that is used by so many packages later and it is generally
>> a bad practice to put stuff into the main fixture if they are only
>> needed for a couple of tests).
>>
>> Therefore we should have a close look at the p.a.contenttypes and
>> p.a.event test fixture and make sure they are as lightweight as possible.
>>
>> I agree that we need to write down some guidelines for tests and that we
>> have to work on that. Though, my priorities right now are the control
>> panels, because they are blocking the Plone 5 beta release. If anybody
>> wants to work on that I'm happy to help though!
>>
>> Timo
> 


More information about the Framework-Team mailing list