[Proposal] Best practice: tests must not output anything outside of the target directory

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[Proposal] Best practice: tests must not output anything outside of the target directory

vmassol
Administrator
Hi devs,

I was pretty sure that this was discussed before but I can’t find it so I’m posting it again since I noticed that we continue to have tests (and even add new ones) that use the OS tmp directory to output test data.

Tests must not generate anything outside of the target/ directory as this will cause several problems:
* not clean generated test data after the test
* create a state that can make other tests fail
* generate errors in jenkins since jenkins monitors created files and doesn't allow to remove files outside of the worskspace

WDYT?

If ok I’ll add it to https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HBestpractices

Thanks
-Vincent

PS: Ideally we should have a automatic verification in the build but it doesn’t seem easy to implement, so right now, I’m only proposing it as a best practice.
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Best practice: tests must not output anything outside of the target directory

Simon Urli
Hi Vincent,

On 01/08/2019 10:35, Vincent Massol wrote:

> Hi devs,
>
> I was pretty sure that this was discussed before but I can’t find it so I’m posting it again since I noticed that we continue to have tests (and even add new ones) that use the OS tmp directory to output test data.
>
> Tests must not generate anything outside of the target/ directory as this will cause several problems:
> * not clean generated test data after the test
> * create a state that can make other tests fail
> * generate errors in jenkins since jenkins monitors created files and doesn't allow to remove files outside of the worskspace
>
> WDYT?
>
> If ok I’ll add it to https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HBestpractices

I agree on the principle, but before adding it we need a utility method
to know where to put those resources: I guess it would return a relative
path to the target directory of the current module, but we need this
method to avoid having to rely on explicit paths in our tests.

It might also allow us to clean automatically those paths in a tearDown
method if we want.

Simon
>
> Thanks
> -Vincent
>
> PS: Ideally we should have a automatic verification in the build but it doesn’t seem easy to implement, so right now, I’m only proposing it as a best practice.
>

--
Simon Urli
Software Engineer at XWiki SAS
[hidden email]
More about us at http://www.xwiki.com
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Best practice: tests must not output anything outside of the target directory

Thomas Mortagne
Administrator
Big +1 for this best practice of course.

And indeed it would be nice to have a share test tool (a junit 5
extension probably) to get a temporary test folder to work and which
is consistent.

What I do in all the tests that needs such a folder is:

        File testDirectory = new File("target/test-" + new
Date().getTime()).getAbsoluteFile();

On Thu, Aug 1, 2019 at 10:43 AM Simon Urli <[hidden email]> wrote:

>
> Hi Vincent,
>
> On 01/08/2019 10:35, Vincent Massol wrote:
> > Hi devs,
> >
> > I was pretty sure that this was discussed before but I can’t find it so I’m posting it again since I noticed that we continue to have tests (and even add new ones) that use the OS tmp directory to output test data.
> >
> > Tests must not generate anything outside of the target/ directory as this will cause several problems:
> > * not clean generated test data after the test
> > * create a state that can make other tests fail
> > * generate errors in jenkins since jenkins monitors created files and doesn't allow to remove files outside of the worskspace
> >
> > WDYT?
> >
> > If ok I’ll add it to https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HBestpractices
>
> I agree on the principle, but before adding it we need a utility method
> to know where to put those resources: I guess it would return a relative
> path to the target directory of the current module, but we need this
> method to avoid having to rely on explicit paths in our tests.
>
> It might also allow us to clean automatically those paths in a tearDown
> method if we want.
>
> Simon
> >
> > Thanks
> > -Vincent
> >
> > PS: Ideally we should have a automatic verification in the build but it doesn’t seem easy to implement, so right now, I’m only proposing it as a best practice.
> >
>
> --
> Simon Urli
> Software Engineer at XWiki SAS
> [hidden email]
> More about us at http://www.xwiki.com



--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Best practice: tests must not output anything outside of the target directory

vmassol
Administrator
In reply to this post by Simon Urli
Hi Simon,

> On 1 Aug 2019, at 10:43, Simon Urli <[hidden email]> wrote:
>
> Hi Vincent,
>
> On 01/08/2019 10:35, Vincent Massol wrote:
>> Hi devs,
>> I was pretty sure that this was discussed before but I can’t find it so I’m posting it again since I noticed that we continue to have tests (and even add new ones) that use the OS tmp directory to output test data.
>> Tests must not generate anything outside of the target/ directory as this will cause several problems:
>> * not clean generated test data after the test
>> * create a state that can make other tests fail
>> * generate errors in jenkins since jenkins monitors created files and doesn't allow to remove files outside of the worskspace
>> WDYT?
>> If ok I’ll add it to https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HBestpractices
>
> I agree on the principle, but before adding it we need a utility method to know where to put those resources: I guess it would return a relative path to the target directory of the current module, but we need this method to avoid having to rely on explicit paths in our tests.

Yes, I agree. I can write a junit5 extension similar to https://junit.org/junit5/docs/current/user-guide/#writing-tests-built-in-extensions-TempDirectory (this one will use the system tmp dir).

> It might also allow us to clean automatically those paths in a tearDown method if we want.

I wouldn’t do this. It’s important to leave the tmp dir present and not touch target/ , fro debugging purposes. In any case target lifecycle is already taken care of when we do a mvn clean.

Thanks
-Vincent

>
> Simon
>> Thanks
>> -Vincent
>> PS: Ideally we should have a automatic verification in the build but it doesn’t seem easy to implement, so right now, I’m only proposing it as a best practice.
>
> --
> Simon Urli
> Software Engineer at XWiki SAS
> [hidden email]
> More about us at http://www.xwiki.com

Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Best practice: tests must not output anything outside of the target directory

Marius Dumitru Florea
+1

On Thu, Aug 1, 2019 at 12:01 PM Vincent Massol <[hidden email]> wrote:

> Hi Simon,
>
> > On 1 Aug 2019, at 10:43, Simon Urli <[hidden email]> wrote:
> >
> > Hi Vincent,
> >
> > On 01/08/2019 10:35, Vincent Massol wrote:
> >> Hi devs,
> >> I was pretty sure that this was discussed before but I can’t find it so
> I’m posting it again since I noticed that we continue to have tests (and
> even add new ones) that use the OS tmp directory to output test data.
> >> Tests must not generate anything outside of the target/ directory as
> this will cause several problems:
> >> * not clean generated test data after the test
> >> * create a state that can make other tests fail
> >> * generate errors in jenkins since jenkins monitors created files and
> doesn't allow to remove files outside of the worskspace
> >> WDYT?
> >> If ok I’ll add it to
> https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HBestpractices
> >
> > I agree on the principle, but before adding it we need a utility method
> to know where to put those resources: I guess it would return a relative
> path to the target directory of the current module, but we need this method
> to avoid having to rely on explicit paths in our tests.
>
> Yes, I agree. I can write a junit5 extension similar to
> https://junit.org/junit5/docs/current/user-guide/#writing-tests-built-in-extensions-TempDirectory
> (this one will use the system tmp dir).
>
> > It might also allow us to clean automatically those paths in a tearDown
> method if we want.
>
> I wouldn’t do this. It’s important to leave the tmp dir present and not
> touch target/ , fro debugging purposes. In any case target lifecycle is
> already taken care of when we do a mvn clean.
>
> Thanks
> -Vincent
>
> >
> > Simon
> >> Thanks
> >> -Vincent
> >> PS: Ideally we should have a automatic verification in the build but it
> doesn’t seem easy to implement, so right now, I’m only proposing it as a
> best practice.
> >
> > --
> > Simon Urli
> > Software Engineer at XWiki SAS
> > [hidden email]
> > More about us at http://www.xwiki.com
>
>