[Avocado-devel] Tests stable tmpdir

Thu Oct 27 17:00:53 UTC 2016

On 10/25/2016 12:15 PM, Ademar Reis wrote:
> On Mon, Oct 24, 2016 at 06:14:14PM -0300, Cleber Rosa wrote:
>>
>> On 10/24/2016 10:27 AM, Amador Pahim wrote:
>>> Hello,
>>>
>>> I saw a number of requests about setUpClass/tearDownClass. We don't
>>> actually support them in Avocado, as already stated in our docs, but
>>> most of the requests are actually interested in have a temporary
>>> directory that can be the same throughout the job, so every test can
>>> use that directory to share information that is common to all the
>>> tests.
>>>
>>> One way to provide that would be exposing the Job temporary directory,
>>> but providing a supported API where a test can actually write to
>>> another test results can break our promise that tests are independent
>>> from each other.
>>>
>>
>> Yes, the initial goal of a job temporary directory is to prevent clashes
>> and allow proper cleanup when a job is finished.  For those not familiar
>> with the current problems of (global) temporary directories:
>>
>> https://trello.com/c/qgSTIK0Y/859-single-data-dir-get-tmp-dir-per-interpreter-breaks-multiple-jobs
> 
> Also, let's keep in mind that the architecture of Avocado is
> hierarchical and tests should not have access or knowledge about
> the job they're running on (I honestly don't know how much of
> this is true in practice today, but if it happens somewhere, it
> should be considered a problem).
> 
> Anyway, what I want to say is that we should not expose a job
> directory to tests.
> 

I believe we have to be clear about our architecture proposal, but
honest also about how we currently deviate from it.  Avocado-VT, for
instance, relies on the temporary dir that exists across tests.

>>
>>
>>> Another way that comes to my mind is to use the pre/post plugin to
>>> handle that. On `pre`, we can create a temporary directory and set an
>>> environment variable with the path for it. On `post` we remove that
>>> directory. Something like:
>>>
>>> ```
>>> class TestsTmpdir(JobPre, JobPost):
>>>     ...
>>>
>>>     def pre(self, job):
>>>         os.environ['AVOCADO_TESTS_TMPDIR'] = tempfile.mkdtemp(prefix='avocado_')
>>>
>>>     def post(self, job):
>>>         if os.environ.get('AVOCADO_TESTS_TMPDIR') is not None:
>>>             shutil.rmtree(os.environ.get('AVOCADO_TESTS_TMPDIR'))
>>> ```
>>>
>>> Thoughts?
>>>
>>
>> I think this can be a valid solution, that promises very little to
>> tests.  It doesn't break our assumption of how tests should not depend
>> on each other, and it reinforces that we aim at providing job level
>> orchestration.
> 
> Thinking from the architecture perspective once again, this is a
> bit different from what you proposed before, but not that much
> (let's say it's a third-party "entity" called
> "AVOCADO_TESTS_TMPDIR" available to all processes in the job
> environment, unique per job).
> 
> It's a bit better, but first of all, it should be named,
> implemented and even enabled in a more explicit way to prevent
> users from abusing it.
> 

This kind of proposal is really a short (or mid) term compromise.  We
don't want to endorse this as part of our architecture or propose that
tests are written to depend on it.  Still, we can't at the moment, offer
a better solution.

Shipping it as a contrib plugin, can help real users to have better
tests.  Not optimal or perfect ones, but still better than what can be
honestly done Today.

> But my real solution is below:
> 
>>
>> Although, since we have discussed giving a job its own temporary dir,
>> and we already expose a lot via environment variables to tests:
>>
>> http://avocado-framework.readthedocs.io/en/latest/WritingTests.html#environment-variables-for-simple-tests
>>
>> And also to job pre/post script plugins:
>>
>> http://avocado-framework.readthedocs.io/en/latest/ReferenceGuide.html#script-execution-environment
>>
>> I'm afraid this could bring inconsistencies or clashes in the very near
>> future.  What I propose for the immediate terms is to write a
>> contrib/example plugin, that we can either fold into the Job class
>> itself (giving it a real temporary dir, with variables exposed to test
>> processes) or make it a 1st class plugin.
>>
>> How does it sound?
> 
> If we expose something like this as a supported API, we should
> make it as an "external resource available for tests" with proper
> access control (locking) mechanisms. In other words, this feature
> is a lot more about the locking API than about a global directory
> for tests.
> 
> In summary, a "job/global directory available to all tests"
> should in fact be handled as "a global resource available to all
> tests".  Notice it has no relationship to jobs whatsoever.
> Creating it per-job would be simply an implementation detail.
> 

That indeed becomes true when we start offering the locking mechanism.
Right now, our users simply want/need to do setup that is valid for many
(usually all) tests.  According to Amador, the lack of a such a
mechanism, has led users to write larger tests, when they should really
be smaller ones.

> Think of the hypothetical examples below and consider the
> architectural implication:
> 
> (all tests in these examples are making use of the shared dir)
> 
> $ export MY_DIR=~/tmp/foobar
> $ avocado run my-test.py
> $ avocado run my-test1.py my-test2.py
> $ avocado run my-test.py & avocado run my-test.py
> $ avocado run --enable-parallel-run my-test*.py
> 
> Some of the above will break with today's Avocado. Now imagine we
> provide a locking API for shared resources. Tests could then do
> this:
> 
>     lock($MY_DIR)
>       do_something...
>     unlock($MY_DIR)
> 
> Or maybe even better, simply declare they're using $MY_DIR during
> their entire execution via a decorator or some (future)
> dependency API:
> 
>     @using($MY_DIR)
>     def ...
> 
> With that in place, we could have a plugin, or even a first-class
> citizen API, to create and expose a unique directory per job.
> 

I think we're really in sync wrt the optimal architecture.  What we need
to come up to an agreement is if/how we deliver a short/mid term
compromise alternative.

> Thanks.
>    - Ademar
> 

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20161027/659ffc84/attachment.sig>