[Piglit] [PATCH 1/2] framework/programs/run.py: Delete tests directory if it exists

Daniel Vetter daniel at ffwll.ch
Fri Aug 7 06:05:17 PDT 2015


On Tue, Aug 04, 2015 at 02:54:56PM -0700, Dylan Baker wrote:
> > On Tue, Aug 4, 2015 at 2:39 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> >
> > Doesn't this have the effect of accidentally removing old results you
> > might want to keep if you do a type (with e.g. shell completion)? Old
> > piglit refused to run if the results dir was there iirc, so maybe we need
> > a --force-overwrite option or similar?
> > -Daniel
> 
> Wow, I should not write emails right after I get up, sigh.
> Here's a better version of that email:
>  
> For summary it does fail if the results already exist, but for run I don't
> think it ever has required --overwrite option.

Indeed I quickly checked an old version of piglit and piglit-run happily
overwrites files there too. No concern from my side, but the extended
commit message would indeed be nice ;-)
-Daniel

> 
> The bug this patch attempts to fix is one introduced by the transition
> from having a single open file to using one file per test to atomicity
> in piglit.  Since we moved to JSON, the results file would be opened,
> each test would be written into it serially, and at the end it would be
> closed.
> 
> After the transition to atomicity we write a single file per test and
> then combine them into a single JSON file at the end. 
> 
> This leads to some odd bugs if you do something like this:
> `piglit run quick deqp-gles3 foo -c`
> 
> let it run, so 35,000 tests, and then stop it with ctrl+\, and then do:
> `piglit run quick foo -c`
> 
> Since the quick profile only as ~25000 tests IIRC, you'll end up with up to
> 10,000 tests from the original run scooped up into the new run, leading to
> very odd bugs. So we remove then at the start. In fact, the only way
> these files should be used is during a `piglit resume` since they are
> deleted after a succesful completion of `piglit run`
> 
> This doesn't change the fact that results.json will be overwritten if you
> do this. If we want to have a --overwrite switch we can do that, but that's
> a separate change.
> 
> Does that make sense? Do I just need to change the commit message to be
> clearer?
> 
> Sorry for the awful email.
> 
> -Dylan



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Piglit mailing list