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

Revision history for this message
Evan (ev) wrote :

We have always said that charms should be stand-alone and not depend on behaviour that only exists in lp:uci-engine. I think by putting the namespace of the charm in the filename, we move away from that. Pretty much every other charm-helpers using charm has hooks/hooks.py and now we have hooks/wsgi_app_hooks.py.

I also think the risks that Vincent highlights are very real and if they were to bite us, would be extremely hard to pinpoint.

I am almost inclined to agree with Francis that it's time to split the charms out into their own branches. We understand the various pieces of this project enough now to be able to accomplish that without branch landing being delicate. However, it's no small amount of work. Each charm requires a new stanza in Tarmac, a new branch for everyone on the team to watch MPs on, and so on.

I am also convinced that it greatly reduces visibility on these ancillary branches. It means that we sacrifice consistency, which already needs to be carefully managed in a microservices architecture.

Instead, I'd like to propose that we isolate the process environment for the charm tests. We fork a child process under the charm directory and run through the tests in complete isolation. The parent process can take the piped output from the child and combine it with its own results.

We've got more opinions on this one than we have people. Let's take it down a level and understand that there are tradeoffs with all approaches.

« Back to merge proposal