[Avocado-devel] Pre/Post Mini-RFC

Thu Mar 30 06:31:19 UTC 2017

Dne 29.3.2017 v 23:10 Cleber Rosa napsal(a):
> ===================
>  Pre/Post Mini-RFC
> ===================
>
> This is a status report on the various Pre/Post like interfaces we
> have for Jobs and Tests.
>
> The goal is to identify what's currently badly defined, and fix the
> interface and implementations so that their goals are clearly
> understandable.  It should set the tone for other new pre/post
> interfaces.
>
> As we start to define a road map for a new LTS release, it's essential
> to eliminate any source of confusion to users and to Avocado
> contributors.
>
> Pre, as in previous to its execution
> ====================================
>
> So far, Avocado has implemented some "Pre" interfaces that are actually
> executed by the subject itself.  This is, by definition, very wrong.
>
> An analogy may be useful.  Suppose we are hosting a car race.  Now
> suppose we have a checklist to run *before* the race, such as checking
> the conditions of the asphalt.  One would never assume, rightfully so,
> that this would be performed as *part of the race*, so no race cars
> would be involved in checking the asphalt conditions.  Instead, the
> race organizers, would do that, before showing the competitors the
> green light.
>
> It's OK, though, for the race organizers to know about the race
> conditions (duration, rules for pit stops, etc) and the cars, so that
> the most adequate conditions are ensured.
>
> If we were writing code that would map this analogy, we would have::
>
>   class Organizer(object):
>       def pre_race(self, race_conditions):
>           check_asphalt_conditions()
>
>   class Race(object):
>       def warmup_lap(self):
>           send_pace_car(speed=50)
>
> It would be confusing, to have something like this::
>
>    class Race(object):
>        def pre_race(self):
>           prepare_a_race_within_a_race_is_wrong()
>
> Or even::
>
>    class Race(object):
>        def pre(self):
>           """
>           Pre what?
>           """
>           prepare_a_err_dunno()
>
> But it would be OK to have::
>
>   class Race(object):
>        def pre_pit_stop(self):
>            notify_mechanics()
>
> Example with current Avocado
> ============================
>
> Avocado has a plugin interfaces called "JobPre".  It's described as
> "Base plugin interface for adding actions before a job **runs**".
> This is clearly wrong, because it's executed as part of the
> ``avocado.core.Job.pre_tests()`` method
> (https://github.com/avocado-framework/avocado/blob/47.0/avocado/core/job.py#L423)
> ::
>
>   def pre_tests(self):
>     ...
>     self._job_pre_post_dispatcher.map_method('pre', self)
>     ...
>
> And it's even more incorrect because ``avocado.core.Job.pre_tests()``
> is called as part of ``avocado.core.Job.run()``
> (https://github.com/avocado-framework/avocado/blob/47.0/avocado/core/job.py#L476)
> ::
>
>   def run(self):
>       ...
>       try:
>           self.create_test_suite()
>           self.pre_tests()
>       ...
>
> Now, **another** interface, called "JobPreTests" exists, and is
> described as "Base plugin interface for adding actions before a job
> runs tests", which is fine, and it gets executed at
> ``avocado.core.job.pre_tests()``.
>
> Proposal
> ========
>
> The proposal is to have "Pre" interfaces that are external to the
> subjects.
>
> This means that it's OK to have "JobPreTests", where a job does
> something before a test (is executed).  For further clarification,
> such an interface could even be called "JobPreTestsExecution", but it
> seems to add verbosity without adding further value.  Other examples
> may actually benefit from being more precise.
>
> With regards to "JobPre", it is a valid requirement, but should
> never be implemented within the Job.  A "JobPre", is, in reality, a
> "JobInit" or "JobSetUp", and should be named as such.  Just think
> of a "job initializing **itself**", or "setting up **itself**".
>
> If the Avocado command line application creates a Job, then it should
> be the one calling a "JobPre" implementations, with enough information
> it has about the upcoming job.
>
> Right now, it may be difficult to do this in Avocado, as a lot of the
> configuration about a Job exists solely within an already instantiated
> Job.  But this is actually a good thing, because the separation of
> configuration coming from sources such as files, command line,
> parameter system and the Job itself is one of the requirements of the
> much talked about "Job API".
>
> For users of the Job API, utility methods could exist, but would still
> require actions/code such as the following pseudo-code::
>
>   from avocado import Job
>   from avocado import load_default_config
>   from plugins.pre_job import run_pre_job
>
>   load_default_config()
>   run_pre_job()
>   job = Job()
>   run_post_job()
>
> For a "JobInit" or "JobSetUp" interface, they could be automatically
> called before a Job is considered ready to "run()".
Hello Cleber, I fully agree with the `Job{Pre,Post}` confusion you 
identify here, it's basically the same thing I was complaining in 
https://trello.com/c/W58vhyHR/539-some-avocado-test-methods-and-attributes-are-public 
about `avocado.Test` but in Job context. Anyway there is a minor 
difference as currently we do not have a need for `Job{Pre,Post}` 
plugins, we only need `{Pre,Post}Tests` as we often need many 
information from job itself, so the change would actually be a bit 
simpler and that is to rename the `Job{Pre,Post}` to `{Pre,Post}Tests` 
or simply implement the `Job{Pre,Post}` correctly from the main app (as 
you described) while adding new `{Pre,Post}Tests` dispatcher which'd 
work exactly as the old `Job{Pre,Post}`.

This actually reminded me the issue Huawei described, they are using a 
custom `Run` plugin and they claim they can't use `Job{Pre,Post}`. This 
could mean they really need to do something before/after the Job 
initialization so the later would probably help them.

Kind regards,
Lukáš

>
> References
> ==========
>
> This (mini) RFC is intended to shape the direction of a few (long
> standing) pull requests:
>
>  1) https://github.com/avocado-framework/avocado/pull/1617
>  2) https://github.com/avocado-framework/avocado/pull/1829
>
> As an RFC, feedback is **extremely** welcome!
>

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