creadur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1617205 - /creadur/rat/trunk/apache-rat-plugin/src/test/java/org/apache/rat/mp/RatCheckMojoTest.java
Date Mon, 11 Aug 2014 12:15:20 GMT
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 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

Mime
View raw message