Code review comment for lp:~cprov/uci-engine/1335753-glance-creds

Revision history for this message
Vincent Ladeuil (vila) wrote :

> The fact that we have tests that depend on 'unit_config' on the project root
> is wrong, in fact.
> If it's not committed in the VCS it should be handled ad-
> hoc by a testing fixture.

There is more than one way to do it :)

The tests that rely on unit_config do use fixtures but still let the user decide whether or not he want to run those tests by creating that file or not.

If you have a better way that doesn't break the existing tests, I'm fine with that.

>
> Also, the only test that needs to exercise disk operations with 'unit_config'
> is the one I've added in this branch, all the rest of the system should mock
> get_auth_config() and use whatever result they expect.

Yes, that's the point I raised when you introduced deploy.install_unit_config()... I don't know which tests required that.

« Back to merge proposal