[cairo] [PATCH v3 0/7] test: Don't ignore test output files when written to the wrong directory
Bryce W. Harrington
b.harrington at samsung.com
Sun Sep 8 10:46:47 PDT 2013
On Sun, Sep 08, 2013 at 03:03:18PM +0200, Uli Schlachter wrote:
> I started merging this and got up to patch 6/7, then stopped. Some comments below.
> On 15.08.2013 20:08, Bryce W. Harrington wrote:
> > Bryce Harrington (7):
> > test: Don't ignore test output files left in test directory
> > test: Fix several tests to place output files in the output directory
> > test: Use CAIRO_TEST_OUTPUT_DIR for name of the output directory
> > test: concat CAIRO_TEST_OUTPUT_DIR at point of use instead of
> > BASENAME
> The first patch adds a hardcoded "output/" to some file names, the second one
> changes that into a define and the third one completely undoes the first patch
> and moves the addition of the "output/"-prefix from the define into the code.
> I don't see why this is spread over three different commits. I would just squash
> them together into a single one (which then would be about as large as each of
> the individual commits, so splitting this up doesn't produce smaller patches).
> It just seems like these are fixup commits which correct errors from the earlier
> > test: Make cairo_test_mkdir() usable throughout tests
> Why does this copy&paste _cairo_test_mkdir() to cairo_test_mkdir()? Wouldn't it
> be enough to rename _cairo_test_mkdir() and make it non-static (and adding it to
_cairo_test_mkdir() is implemented in indivdual tests. This commit
places it in the core test code so other tests can link to it. You
can't just rename one of the existing routine implementations.
> > test: Ensure output dirs exist, falling back to current dir if needed
> I don't know what I should say. I read the description, then looked at the diff
> and afterward had to read the description again. This is the point were I
> stopped committing the patches, because this makes no sense at all.
> The diff should be squashed into the previous commit and the change described,
> if needed, should be implemented.
> > test: Refer to output filename by variable, not a hardcoded value
> This could get "test, svg:" instead of just "test". Besides that, dunno. Never
> looked closely at how SVG does things.
> Sorry for complaining again (and sorry for being awfully slow for looking at
> things! If you want to, feel free to just do something about patch 6/7 and
> ignore the rest of my complaints, those are not that important).
> Sent from my Game Boy.
More information about the cairo