← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] SHAPE_ARC tests

 

Hi Wayne,

It has an "expected failures" counter. This is so a test can be
written to illustrate a bug without fixing it in the same commit. This
means:

1) You can flag an existing bug up without breaking CI builds with a
failing `make test` at that commit. Ubuntu PPAs, for one, will fail to
build packages if the tests fail.
2) There is a record in the Git log of the error, as well as the fix.
If you squash these commits, there is no record of code that exposed
the bug.
3) You can be sure the test did, in fact, catch the bug in the first
place. More importantly, a code reviewer can see it too. If you squash
it, you won't know, other than trusting that the programmer did cause
the failure before fixing the bug.

This rationale and the way to do it is documented in the dev docs [1].
This has been done previously in
ee819216e2523019f6f5bbca0a184d13ac381f1a for COLOR4D for the same
reason.

WRT 8 spaces: this is what clang-format has been configured to do with
ContinuationIndentWidth.

Drawing bounding boxes sounds like a handy advanced config option!

Cheers,

John

[1]: http://docs.kicad-pcb.org/doxygen/md_Documentation_development_testing.html#expected-failures

On Wed, Jan 9, 2019 at 4:52 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> John,
>
> If I only apply patch 1, shouldn't the arc bounding box test fail?  I
> just tried this and all of the tests passed.
>
> Wayne
>
> On 1/9/2019 11:14 AM, John Beard wrote:
> > Hi,
> >
> > Couple of patches for the SHAPE_ARC geometry class.
> >
> > Notably, a bug in the bounding box code is exposed: the bbox is
> > currently computed to be a box that contains the start, end and
> > centre. This does not work if the arc passes a "quadrant point". This
> > is not an issue for 90-deg arcs, but it means, for example, the bbox
> > for a 180 degree arc is zero-area, as the centre and both endpoints
> > are collinear. This could be related to occasional rumours of pads
> > disappearing when at high zoom (?)
> >
> > The second patch contains a computation for it and removes the
> > expected failures. It would be good to get a review to check I've done
> > it correctly! Specifically: have I missed an important case in the
> > tests?
> >
> > Also includes a couple of handy geometry test predicates for vectors and boxes.
> >
> > Cheers,
> >
> > John
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> >
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp


Follow ups

References