checkbox-dev team mailing list archive

[Zygmunt Krynicki <zygmunt.krynicki@xxxxxxxxxxxxx>]

Hey.

I need some help reviewing the virtual-units branch [1].

[SNIP]

I've made some changes since I've proposed it for early feedback a few
days ago. The main changes since last time are:

- I've changed all of the code to use new APIs. This is described in
detail by this commit [2]
- I've *removed*, not deprecated the old APIs. After all, when
nobody's calling why keep this code.
- I've added some docstrings here and there.
- I've made the whole branch test out OK at each commit. This was the
painful step.

If you recall, the main idea behind this branch is to combine test
plans and whitelists internally in the API (users can still create
either file and expect the same result). This branch moves us closer
to that goal by creating a TestPlanUnit instance for every .whitelist
file and a WhiteList instance for every loaded test plan unit.

In addition, it makes the provider API a lot easier to use by
replacing all of the verbose methods, often returning a pair of
things, with simple properties for all objects that we care about.
This API change is optional and if there's a desire for that, we can
be actually moved to a second branch to land after this one.

As a bonus, this branch contains code originally written for the
remote testing feature. It allows us to discover all files belonging
to a provider, classify it by role and even re-locate it to a new
directory (and layout) using the base directory property. All of this
is thanks to the FileUnit.

I'm looking for feedback on the following topics:
 - design of the new provider loader stack
(ProviderContent{Enumerator,Classifier,Loader})
 - missing API conversions
 - bugs introduced by the changes :-)

I've done some basic testing but nothing elaborate. I mainly relied on
unit tests.

Thanks
ZK

[1] https://code.launchpad.net/~zkrynicki/checkbox/virtual-units/+merge/248929
[2] http://bazaar.launchpad.net/~zkrynicki/checkbox/virtual-units/revision/3583