[Avocado-devel] Implement fetch-assets command line

[Cleber Rosa]

On Thu, Sep 26, 2019 at 10:08:47AM +0200, Amador Pahim wrote:
> On Thu, Sep 26, 2019 at 9:49 AM Lukáš Doktor <ldoktor at redhat.com> wrote:
> >
> > Hello guys,
> >
> > what a nice discussion. But before you start, what lead you to pick this card? It's one of the nice-to-have-ideas card and we have plenty of well-defined-and-useful cards that also needs attention, so unless you have a real-world usage, I'd probably suggest to focus on something you can directly benefit from. (unless you have other interest in eg. learning something, or other kind of interest).
> 
> Fair point. A RFC would do some good here.
>

Expanding the description on the card and linking the QEMU threads
would be a good idea too.

> >
> > Dne 25. 09. 19 v 23:39 Cleber Rosa napsal(a):
> > > On Wed, Sep 25, 2019 at 06:01:22PM -0300, Willian Rampazzo wrote:
> > >> After going thru the suggestions, and thinking about it, I came up
> > >> with the following starting point:
> > >>
> > >> 1 - Support a limited test source code parsing. This feature brings a
> > >> simple solution to those tests already using strings into their
> > >> fetch_asset parameters, as stated by Cleber.
> > >>
> > >
> > > Keep in mind that I'm assuming this is a viable option without
> > > attempting to do it myself.  I hope it's viable, because it makes
> > > "simple things easy" from the test writer's side.
> > >
> >
> > Yes, this makes sense to me and would be useful even for some human-asset interface (list assets, list cached assets, see details of cached assets...). Btw cache-management interface is IMO higher in the priority than actually fetching the assets without executing tests (but it's just my priority list, don't take this as an official statement).
> >
> > >> 2 - Add an option to use assets.json, as stated by Amador. This file
> > >> would sit in the test data folder and list all assets that are
> > >> necessary for the test. As a list of assets, I thought about a new
> > >> method, fetch_assets, as a wrapper that load all assets from the file
> > >> and call fetch_asset for each one.
> > >>
> > >
> > > Sounds good.  Also consider the need for dynamic asset definition (the
> > > idea about executable scripts that produces JSON).
> > >
> >
> > Hmm, this is tricky. I don't like much having "reserved" file names as people might get unwanted clashes. Yes, it should be fairly harmless with a json file (either it matches or is skipped) but it's a semi-hidden feature. We also have the https://trello.com/c/WPd4FrIy/1479-add-support-to-specify-assets-in-test-docstring that sounds more explicit to me. Another alternative I'd prefer is a `Test` class variable, which can be obtained by a static analysis, or even `Test` variable initialized on `__init__`, which could be executed by the asset fetcher interface (when asked for, because it might eventually execute some nasty code). Having a pre-defined file names in datadir locations is the last interface I'd like to see in Avocado (out of these).
> >
> 
> I don't think reserved filenames are a good way either. The baseline
> from my suggestion is an external file containing the arguments that
> will be used by both the asset.Asset(from_file=filename.json) and by
> the command line tool that will cache things: avocado fetch-asset
> filename.json filename2.json filename3.json.
> 
>

I disagree, because I am big a believer in well known conventions.

Just for the sake of comparison, we can't have "#!" as the first chars
in an executable file and not have the following characters treated as
the interpreter because of a convention.  To miminize clashes, we need
to pick good names, for instance, I find it hard that someone will
*have to* name a file "avocado_assets.json" inside a directory named
"$MY_TEST.data" for any other reason.

Finally, if that's still a valid scenario, a configuration for the
naming can be defined and activated for one particular installation,
user or execution.  The number of "normal" occurrences though, and the
saved time and convenience of those conventions, IMO, prove
themselves.

> > As before, if you have some real-world use case, please share it as it can change everything.
> >
> >
> > class variable
> > --------------
> >
> >     class MyTest(avocado.test):
> >         assets = {"foo": ["http://aaa"], "bar": {"location": "http://fdsa", hash="..."}}
> 
> This seems like a good alternative, but it also seems like one more
> source or parametrization. I'd then use the parametrization interface
> instead.
>
> >
> > initialized variable
> > --------------------
> >
> >     class MyTest(avocado.test):
> >         def __init__(self, ...):
> >             self.assets = {}
> >             for asset in something:
> >                  self.assets[asset] = get_asset_url(asset)
> 
> I would not ask users to deal with __init__ in their test classes.
>

I agree, the Avocado.Test interface for __init__ is ugly and users
should as much as possible be kept away from it.

- Cleber.

> >
> > ---
> >
> > Regards,
> > Lukáš
> >
> 
> 
> -- 
> Pahim