directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: svn commit: r1395286 - in /directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core: authn/ authn/ppolicy/ authz/ jndi/ operations/add/ suites/
Date Sun, 07 Oct 2012 18:08:01 GMT
Le 10/7/12 6:59 PM, Stefan Seelmann a écrit :
> On 07.10.2012 17:22, Emmanuel Lécharny wrote:
>> Hi Stefan ! Glad to see you around :)
>>
>> Le 10/7/12 3:03 PM, Stefan Seelmann a écrit :
>>> On 07.10.2012 14:26, elecharny@apache.org wrote:
>>>> Author: elecharny
>>>> Date: Sun Oct  7 12:26:32 2012
>>>> New Revision: 1395286
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1395286&view=rev
>>>> Log:
>>>> Added a sysout to know which test is being processed (it's useful
>>>> when running the suite, as the tests are quite long)
>>>>
>>>> Modified:
>>>>      
>>>> directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/suites/MigratedStockCoreISuite.java
>>>>
>>>>      
>>>> directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/suites/StockCoreISuite.java
>>>>
>>> Maybe a dumb question but why not just remove the suites completely?
>> What I'm doing currently is to migrate the JNDI suites to use the API
>> instead (but keeping the pure JNDI tests just to be sure that we still
>> can access to the server using JNDI) to the Migrated suite. Once done,
>> I'll remove the StockCoreISuite. But this is a bit orthogonal to your
>> question...
>>
>> The reason we have the suite is because we can define some suite related
>> server, instead of having to declare that for every single class. At
>> least, this was the rational. Is it still useful ? That's
>> questionable... See later.
>>
>>> In pom.xml I replaced the current inclusion of the suites:
>>>
>>> -                <include>**/*ISuite.java</include>
>>> +                <include>**/*IT.java</include>
>>> +                <include>**/*Test.java</include>
>>>
>>> The result:
>>> * The build of core-integ does not take longer, rather it is some
>>> seconds faster!
>>> * 711 tests were run instead of 684 when using the suites, so it seems
>>> some test classes are missing in the suite classes.
>> Once upon a time, the IT tests were only ran when you use the
>> 'integration' profile (thus the -Dintegration parameter in the command
>> line).
>>
>> I'm not sure this is a good idea to have this flag anymore : all those
>> tests are mandatory, and needed to be sure that the server works, and I
>> personally always use this flag *before* committing.
>> So, IMO, we may make this a default build profile, and run all the tests
>> as you declared them (ie the double include).
> In fact it is already activated by default. The -Dintegration flag has
> no effect, the integration tests run even without that flag.
Doh...
>
> Looking into the core-integ/pom.xml there are three profiles for tests:
> * integration: activated by default, runs the (two) suites
> * integration-all-partitions: activated with -Dtest-all-partitions, runs
> the suites 3 times with JDBM, AVL, and LDIF partition, fails currently,
> not sure if this is useful
I guess it was added when you were working on the HBase partition, just 
in case we wanted to add some new partitions. Don't remember exactly the 
context...
> * quicktests: activated with -Dquicktest, only runs 9 tests, not at all
> useful IMO. Should it be removed?
IMO, yes.
>
>> I'd like to push it a bit further : do we *really* need the suite system
>> at all ? I mean, we want to be able to run one test or another in
>> eclipse while debugging, so everything we declare in a suite annotation
>> will just be overriden by the annotations we have in each tests : this
>> make it doubtful that the suite annotations are useful at all... Not to
>> mention that it makes the annotation's handling way more simple if we
>> don't have to handle the suites...
> I agree.
>
> And I see another problem with suites: It happens that one forgets to
> add a new test class ot the @Suite.SuiteClasses list. As surefire is
> configured to only run the suites (<include>**/*ISuite.java</include>)
> the not added test class won't run. IMO suites are more harmful than useful.
I agree.

I can remove them if nobpdy objects.

FYI, I'm waiting for MINA 2.0.7 to be released (next tuesday) to start 
cutting a new ADS release. So I have 1 full day to spend on such a task.


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com


Mime
View raw message