creadur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Lundberg <denn...@apache.org>
Subject Re: svn commit: r1617205 - /creadur/rat/trunk/apache-rat-plugin/src/test/java/org/apache/rat/mp/RatCheckMojoTest.java
Date Thu, 14 Aug 2014 20:17:25 GMT
On Mon, Aug 11, 2014 at 2:15 PM, sebb <sebbaz@gmail.com> wrote:
> On 11 August 2014 12:59, Dennis Lundberg <dennisl@apache.org> wrote:
>> On Mon, Aug 11, 2014 at 12:38 PM, sebb <sebbaz@gmail.com> wrote:
>>> On 11 August 2014 11:17, P. Ottlinger <pottlinger@aiki-it.de> wrote:
>>>> Hi Sebb,
>>>>
>>>>
>>>> On 2014-08-11 12:09, sebb wrote:
>>>>>
>>>>> AFAICT the test does not detect the error.
>>>>
>>>>
>>>> yes, that's why I was asking for proposals on how to test it ;-)
>>>>
>>>>
>>>>> I think the problem is that the tests are run in a different environment.
>>>>>
>>>>> There probably needs to be an IT instead to run the code directly.
>>>>> And this commit should probably be reverted, as it does not add anything.
>>>>
>>>>
>>>> I see your point, but my commit ensures that during IT the project and its
>>>> artifact lists is not null.
>>>
>>> OK, then drop the references to the JIRA, as the checks aren't relevant to it.
>>>
>>>> Obviously my test patch didn't catch the original NPE, but my checkin fixed
>>>> the issue for your setup, didn't it?
>>>
>>> Yes, the NPE disappeared.
>>>
>>>> I tried some stackoverflowing but didn't find anything useful. Many people
>>>> complain about the low testability of maven plugins.
>>>
>>> I suspect the issue is documentation.
>>> [I have had a quick look, and there seems to be nothing that explains
>>> how to build IT tests in proper detail.]
>>>
>>> As an experiment, I tried changing
>>>
>>> apache-rat-plugin/src/test/invoker/it1/invoker.properties
>>>
>>> to run the rat goal rather than check
>>>
>>> This produced the NPE when I reverted the fix.
>>> However, the test also fails with the fix, presumably because the goal
>>> output is different.
>>>
>>> I think there needs to be another IT with extra tests, but I have not
>>> created any such items.
>>> It should be possible to copy/adapt another IT test, but getting that
>>> working properly might not be easy owing to the fragmented and
>>> incomplete documentation.
>>
>> I agree. It's better to have many small ITs that each test one thing.
>
> +1
>
>> Let me have a look at the ITs in general, and adding one that catches
>> whatever is causing problems. I've created a decent amount of Maven
>> Plugin ITs in my day.
>
> Great - maybe we can a README to the IT tree to explain how to add new tests?
> Also point to the Maven docs that describe the layout and processing.
>
> For example, I can see that there are test/ and test/invoker
> sub-folders that relate to each other.
> However, it is not clear how the shared files under test/ and
> test/invoker should be used.
> In particular, the files under java - are these used for all it tests?
> If so. how does one allow for different test conditions?

I moved things around a bit to make it clearer which parts belong
together. The integration tests are now under src/it and the unit
style tests are now under src/test/java and src/test/resources. These
locations are the standard locations for these kinds of tests in a
Maven project, see
http://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html

I hope that there is now a clear separation between unit and integration tests.

>
>>> I may give it a try, but no promises!
>
> I added it4_RAT-168
>
> But note that it2 and it3 don't seem to be run currently - RAT-169
> It would be good to fix that.
>
>>>
>>>> Thanks
>>>> Phil
>>>>
>>>>
>>
>>
>>
>> --
>> Dennis Lundberg



-- 
Dennis Lundberg

Mime
View raw message