On Mon, Aug 11, 2014 at 2:15 PM, sebb <[email protected]> wrote: > On 11 August 2014 12:59, Dennis Lundberg <[email protected]> wrote: >> On Mon, Aug 11, 2014 at 12:38 PM, sebb <[email protected]> wrote: >>> On 11 August 2014 11:17, P. Ottlinger <[email protected]> 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
