<div dir="ltr">Please keep all discussions on the mailinglist or PLIP ticket when possible, so the process in transparent to all community members.<div><br></div><div>Cheers!</div><div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 11 December 2014 at 11:51, Timo Stollenwerk <span dir="ltr"><<a href="mailto:tisto@plone.org" target="_blank">tisto@plone.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
I just talked to Philip (as discussed at the last FWT meeting). He will<br>
revert the changes and make a pull request. We should invite him (and<br>
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.<br>
<br>
Cheers,<br>
Timo<br>
<br>
Am 09.12.2014 um 13:39 schrieb Johannes Raggam:<br>
<div class="HOEnZb"><div class="h5">> On Tue, 2014-12-09 at 12:10 +0100, Timo Stollenwerk wrote:<br>
>> Am 09.12.2014 um 11:16 schrieb Philip Bauer:<br>
>>> 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 (<a href="https://github.com/plone/plone.app.contentmenu/pull/8" target="_blank">https://github.com/plone/plone.app.contentmenu/pull/8</a>) 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.<br>
>><br>
>> Thanks for bringing this up! :)<br>
><br>
> yep.<br>
><br>
>> The initial idea was to move all tests to use Dexterity types or the<br>
>> p.a.contenttypes fixture (if necessary). I don't think a generic test<br>
>> fixture that supports both AT and DX is possible.<br>
>><br>
>> Tests with tons of if/else statements for DX/AT are a horrible idea in<br>
>> my optinion. They are a nightmare to maintain, to debug and really hard<br>
>> to understand.<br>
><br>
> In plone.app.event until 1.2 there is a generic test suite for<br>
> Archetypes and Dexterity. I think, this approach was quite elegant:<br>
><br>
> In<br>
> <a href="https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32" target="_blank">https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32</a> 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:<br>
><br>
> <a href="https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13" target="_blank">https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13</a><br>
><br>
> You see, ``TestEventViewDX`` defines all tests cases and the concrete<br>
> implementation of ``event_factory`` and uses an DX test layer.<br>
> ``TestEventViewAT`` below just subclasses the DX one, overrides<br>
> ``event_factory`` and uses an AT layer:<br>
> <a href="https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63" target="_blank">https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63</a><br>
><br>
> So both unit tests classes run the same the test cases.<br>
><br>
> That's possible, because there are AT and DX content type<br>
> accessors/wrappers, which unify ``set`` and ``get`` access to those<br>
> types.<br>
><br>
> Initially this was a proof of concept on how to support both Archetypes<br>
> and Dexterity in one package without duplicating code. It works quite<br>
> well! But we were removing the Archetypes based code from<br>
> plone.app.event 2.0, since this release is Dexterity only.<br>
><br>
> To the question on what to do with our Plone tests: Instead of creating<br>
> accessor methods for AT and DX content types, I think it's better to<br>
> rewrite for Dexterity only and move the Archetypes test classes into the<br>
> ATContentTypes package before.<br>
><br>
> Johannes<br>
><br>
><br>
>> Therefore, even though it is not perfect, I think we should stick with<br>
>> the approach to test against Dexterity only. For some packages it might<br>
>> be necessary to actually test both, but those should be the exception<br>
>> from the rule. Most packages just need "some" content object and it does<br>
>> not really matter if it is a DX or AT object.<br>
>><br>
>> One problem I see with that approach is that the p.a.contenttypes<br>
>> fixture, that most packages use, might be a bit too heavy. If<br>
>> p.a.contenttypes becomes the de-factor test layer we have to make sure<br>
>> that we keep it lightweight (because it is incredibly hard to change a<br>
>> test fixture that is used by so many packages later and it is generally<br>
>> a bad practice to put stuff into the main fixture if they are only<br>
>> needed for a couple of tests).<br>
>><br>
>> Therefore we should have a close look at the p.a.contenttypes and<br>
>> p.a.event test fixture and make sure they are as lightweight as possible.<br>
>><br>
>> I agree that we need to write down some guidelines for tests and that we<br>
>> have to work on that. Though, my priorities right now are the control<br>
>> panels, because they are blocking the Plone 5 beta release. If anybody<br>
>> wants to work on that I'm happy to help though!<br>
>><br>
>> Timo<br>
><br>
</div></div><div class="HOEnZb"><div class="h5">_______________________________________________<br>
Framework-Team mailing list<br>
<a href="mailto:Framework-Team@lists.plone.org">Framework-Team@lists.plone.org</a><br>
<a href="https://lists.plone.org/mailman/listinfo/plone-framework-team" target="_blank">https://lists.plone.org/mailman/listinfo/plone-framework-team</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div>Rob Gietema<br><a href="https://www.fourdigits.nl/over-ons#roel-bruggink" target="_blank"><font color="#1155cc">https://www.fourdigits.nl/</font><font color="#1155cc">over-ons#r</font>oel-bruggink</a><br><br>Four Digits BV<br><a href="https://www.fourdigits.nl/" style="color:rgb(17,85,204)" target="_blank">https://www.fourdigits.nl</a> tel: +31 26 4422700<br></div></div></div>
</div>