On Tue, 2010-07-13 at 05:04 -0400, Kamil Paral wrote:
Ah, you got me, I didn't imagine that. I wanted to evaluate the
whole
control file but that would mean I also run "job.run_test(...)" line,
which I don't want to. So I just parsed out aq_* variables.
Ah, that makes sense.
Alright, possible solutions:
1. Make separate file for AutoQA variables, like control.autoqa. We
can then easily evaluate the whole file. But it's another file for
every test... I don't like many files :)
Well, yes, but we need to put test metadata (for 'autoqa') *somewhere*.
(We didn't have one before because right now we only have one piece of
metadata: what hook(s) the test runs under. This is stored in the
'testlist' files for each hook, rather than letting the tests declare
which hook(s) they want to run under, which is somewhat backwards.)
I think we're going to need a general facility for storing autoqa test
metadata. Storing it in the 'control' file seems convenient but
'control' is designed to be interpreted and run by autotest, not autoqa.
The environment / expectations for that file are quite different and
it's hard to separate the autotest parts from the autoqa parts - as
we're finding out.
So. I think the sensible thing to do might be to have a new file where
we can store/override test metadata - things like labels and hooks. Yes,
this means another file for each test (boo! more files!) but if we're
smart about the file format/syntax we won't need to add any others. I
hope.
I understand it. Currently only autoqa script and watchers script
run
on autotest server machine. Now also parts of tests will run there.
But there are these two points:
1. I think we will always review any test that would be accepted and
deployed to our production servers. It's very easy to see any mischief
in the control file. I don't think it is probable some malicious code
could be put in there and we haven't seen it.
I agree, it's not a huge security problem. It's just like RPM specfiles:
technically you can run any shell command you want in them, including
'rm -rf /'. But we review them all, and they get run as a normal user,
in a chroot (under mock), so they aren't generally malicious and can't
do any real damage even if they are.
I'd feel much safer if the autoqa 'control' bits either a) ran as an
unprivileged/restricted user, or b) were a simple template/config file
rather than actual python code to be executed.
2. I have the feeling we can't do without it. We need to use
autotest
labels, right? They must be defined per test, not per hook. Also we
need a dynamic definition ("because test XX runs on YY.fc13 package,
it needs 'fc13' autotest label" - you can hardly define this statically).
Dynamic definition means some scripting language to provide the logic,
you can't get by with just a plain text file or smth. Well and that
means some Python script in the end.
Definitely agree (as suggested above) that we will need some way of
defining per-test variables that autoqa can understand and pass along to
autotest.
And I think you're probably right about needing a certain amount of
scriptability - at least variable interpolation / templating - to get
the job done.
So, given the use cases we've got so far (hooks/labels) and things test
authors might want to tell autoqa in the future: do you think we need
full python scriptability here, or can we get away with a simple config
file with variable interpolation (e.g. labels = ['%(release)', 'virt'])?
> Why vars.copy() instead of just 'for key in vars:'?
Because you can't delete keys from a collection that you're traversing at
the same time. There might be some... side effects :)
Ah, true. But if you're iterating over the keys of a dict, you can do:
for key in vars.keys():
rather than copying the entire dict.
I should finally define some vim shortcuts to fix this up for me...
Yeah, I know James uses a .vimrc config that shows trailing whitespace.
Me, I have:
[color]
diff = auto
status = auto
branch = auto
in my ~/.gitconfig, and 'git diff' shows trailing whitespace in red, so
I just run 'git diff' a lot. Heh.
-w