Code review comment for lp:~raharper/curtin/trunk.vmtest-reuse-output

Revision history for this message
Scott Moser (smoser) wrote :

On Thu, 23 Feb 2017, Ryan Harper wrote:

> On Thu, Feb 23, 2017 at 2:18 PM, Scott Moser <email address hidden> wrote:
>
> > Review: Needs Fixing
> >
> > add documentation of the new environment variable (as you have in the
> > commit message)
> >
>
> ACK
>
>
> >
> > the other question is that this change possibly hides failures ...
> > I'm just reading the diff in line and not thinking too much, but if an
> > error causaed there to be no grub_config or reporting_config, it looks like
> > we'd now just go on, where before we would fail a test.
> >
>
> we avoid *rewriting* files if they exist; I don't think that skips any
> errors;
>
> The basic concept is, if the tempdir has a cls.__name__ directory, to
> rebuild the cls and TempDir paths to files that may get referenced in the
> unittests;
> however, if we're re-using, we don't want to rewrite the contents a second
> time; so all of those checks are if not os.path.exists(foo): write_foo().
>

Thats sane then.
As I said, i just looked quick an thought maybe you were just going to
skip over errors.

« Back to merge proposal