Re: [Qemu-devel] [PATCH 2/2] Acceptance tests: add make rule for running them

[Eduardo Habkost]

On Fri, Sep 21, 2018 at 03:34:02PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > On Fri, Sep 21, 2018 at 08:36:22AM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <address@hidden> writes:
> >> 
> >> > On Thu, Sep 20, 2018 at 11:19:56AM -0400, Cleber Rosa wrote:
[...]
> >> >> +$(TESTS_RESULTS_DIR):
> >> >> +       $(call quiet-command, mkdir -p $@, \
> >> >> +            MKDIR, $@)
> >> >> +
> >> >>  check-venv: $(TESTS_VENV_DIR)
> >> >>  
> >> >> +check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> >> >> +       $(call quiet-command, \
> >> >> +            $(TESTS_VENV_DIR)/bin/avocado \
> >> >> +            --show=none run --job-results-dir=$(TESTS_RESULTS_DIR) 
> >> >> --failfast=on \
> >> >> +            $(SRC_PATH)/tests/acceptance, \
> >> >> +            "AVOCADO", "tests/acceptance")
> >> >
> >> > I think we should provide something easy to use for people who
> >> > already have the right Avocado version installed in their system
> >> > and want to avoid re-downloading Avocado every time.
> >> >
> >> > We already have plans to do this automatically/transparently in
> >> > the future, but maybe while we don't have something automatic we
> >> > could have two separate rules.  e.g.:
> >> >
> >> >   AVOCADO = avocado
> >> >
> >> >   check-acceptance: $(TESTS_RESULTS_DIR)
> >> >          $(call quiet-command, \
> >> >               $(AVOCADO) \
> >> >               --show=none run --job-results-dir=$(TESTS_RESULTS_DIR) 
> >> > --failfast=on \
> >> >               $(SRC_PATH)/tests/acceptance, \
> >> >               "AVOCADO", "tests/acceptance")
> >> >
> >> >   check-acceptance-venv: check-venv
> >> >          $(MAKE) check-acceptance AVOCADO=$(TESTS_VENV_DIR)/bin/avocado
> >> 
> >> Recursion just to override a variable feels like overkill.
> >> Have you considered target-specific variable values?
> >> 
> >> https://www.gnu.org/software/make/manual/html_node/Target_002dspecific.html#Target_002dspecific
> >
> > I have, but I wasn't sure what would be the pitfalls when doing
> > that.
> >
> > For example, the following seems to work but makes the build
> > results depend on the ordering of targets:
> >
> >   $ cat Makefile
> >   VAR=x
> >   a:
> >     echo $(VAR)
> >   b: VAR=b
> >   b: a
> >   $ make a
> >   echo x
> >   x
> >   $ make b
> >   echo b
> >   b
> >   $ make a b
> >   echo x
> >   x
> >   make: Nothing to be done for 'b'.
> >   $ make b a
> >   echo b
> >   b
> >   make: 'a' is up to date.
> >   $
> 
> I doubt that's a problem for practical applications.  Why would anyone
> ask for two conflicting check-acceptance runs, and expect to get *both*?

I think you're right.  I'm just not confident in my understanding
of make to be sure there are no other unexpected pitfalls.  If
you don't see any problem, we can do that.

Anyway, we probably won't need two separate rules in this case if
we use the '--system-site-packages' option of venv.

-- 
Eduardo