[Avocado-devel] Tests stable tmpdir

[Ademar Reis]

On Thu, Oct 27, 2016 at 02:00:53PM -0300, Cleber Rosa wrote:
> 
> 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.

Agree. Avocado-vt is an exceptional case. It's intrusive and
depends on multiple parts of avocado-core, mixing job and test
concepts, even though in theory it's a third-party plugin.

> 
> >>
> >>
> >>> 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.

Agree. I suggest using a name that better represents what this
resource is. Maybe calling it something like
"XXX_TESTS_COMMON_TMPDIR" instead of "AVOCADO_TESTS_TMPDIR".

("XXX" is my attempt to show this is a non-supported variable --
given it's a contrib plugin, users should be free to rename it)

> 
> > 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.

So we can offer the contrib plugin without the locking mechanism,
leaving it to users to decide what to do with it. Documented as a
non-supported feature.

As we learn more about this use-case, we can expose it as a
fully-supported API.

> 
> > 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.

Agree. The contrib plugin sounds OK to me.

Thanks.
   - Ademar

-- 
Ademar Reis
Red Hat

^[:wq!