kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #38941
Re: [PATCH] SHAPE_ARC tests
Hey John,
On 1/9/2019 12:17 PM, John Beard wrote:
> 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.
Thanks for the explanation.
>
> WRT 8 spaces: this is what clang-format has been configured to do with
> ContinuationIndentWidth.
We should probably set this to 4 for the sake of consistency. I'm not
going to hold up merging this patch for this. That can be part of
another patch.
>
> 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
References