Code review comment for lp:~cprov/uci-engine/charm-tests

> Charms are designed to be standalone projects, they just happen to be in the
> same tree because it's convenient to us,

Components are also designed to be stand alone, we know we have issues there
around the overall project layout but we've already taken some steps to
ensure they use different parts of the python name space so their code can
be loaded in a single python process.

It's not about being convenient it's just about using python and respecting
the language definition.

> the same goes for running their tests
> inside a single *super-project* test suite, it's *just* way more convenient
> than spawning isolated `make check`s and collect results.

... and allowing test listing/selection and displaying the timings and being
able to use subunit as an output and being able to run them concurrently

> That said, in order
> to keep the wanted benefits, UCI-E has to compromise.

Says who ?

There *is* a valid fix which is to fix the charms to use their own name space like we do for the components.

>
> We are all aware of the risks involved in reloading modules,

I have no idea about how tricking python into believing it can import
different modules in the same name space spot can manifest itself or mask
issues, that's the whole point.

> but if it is what
> it takes to assure charm tests (and only them) are run as part of our test
> suite, so be it. And by all means, I agree with you on keeping this hack as
> isolated and instrumented as possible, so we will notice if it is behaving
> badly, if it ever does.
>
> > I think it's time to stop using bigger hammers that do more harm than good,
> > step back to the original decision to rely on 'import hooks' and fix that.
>
> It sounds good, but `import hooks` & `import charmhelpers` are not going to
> change,

Of course they can change, I already have one charm fixed to define its own
name space and fix the root issue.

> they make all sense at the charm-level; and we have to ensure we run
> charm tests in tarmac, today, not tomorrow.

The charms should be fixed, not run-tests, changing python semantics is not
run-tests job.

>
> Moreover, It's not a *big hammer*, since it's well isolated (custom loaders
> and suite are only used for charm tests) and the behaviour can be entirely
> overridden by tweaking 2 lines,

Your patch is already ~300 lines long, that's far from 2 lines.

> if we find real, not hypothetical, evidences
> that it's not doing what it is supposed to do.

The real evidence is that python can't load different modules under the same
name.

That's a real issue, perfectly clear and understood. I see no point in
tricking python when there is a way to use it properly.