← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] qa_geometry tests

 

Note to Wayne: Nothing here concerns v5 release, I was just trying to
get a geom test working for future.

On Mon, Jul 9, 2018 at 9:12 AM, John Beard <john.j.beard@xxxxxxxxx> wrote:
> Hi,
>
> Let's come back to the make test behaviour after v5, I think we'll
> need to discuss that separately. However, I think this does illustrate
> why we need the tests to be runnable easily, otherwise they suffer
> bit-rot, and then the tests are useless.
>
> Looking at that change, the test is now iterating the second parameter
> which is now "max error", not "number of segments". Does this test
> still make any sense? It's now comparing:
>
>     SHAPE_POLY_SET::Fillet( radius, error );
>
> with
>
>     CPolyLine::Fillet( radius, segments );
>
> The original test was designed to ensure SHAPE_POLY_SET::Fillet and
> CPolyLine::Fillet were the same, but they now have different
> interfaces and semantics. Wouldn't it be better to check
> SHAPE_POLY_SET::Fillet (and Chamfer) against some ground truth?
>
> Cheers,
>
> John
>
> On Fri, Jul 6, 2018 at 8:40 PM, Nick Østergaard <oe.nick@xxxxxxxxx> wrote:
>> Hi
>>
>> I guess we could add it to the qa target somehow? What I don't particularyly
>> like with this patch is that executing "make test" does not check for
>> dependency changes.
>>
>> Back to the status about qa_geometry... it did pass a long time ago, doing a
>> bit of git bisect points at this commit as the one breaking the test.
>>
>> fbf10e941bdf26bb3618aba0a1b7c44fd0bafed2 is the first bad commit
>> commit fbf10e941bdf26bb3618aba0a1b7c44fd0bafed2
>> Author: Jeff Young
>> Date:   Thu Mar 22 18:02:45 2018 +0000
>>
>>     Switch zone fillets to absolute-error algorithm.
>>
>>     And some general cleanup to related constants, etc.
>>
>> :040000 040000 8b6ad8d44a7b38e0355ce5c8897f823d6255f811
>> 8d54d43a9bd6e5062d6b804890a779e785e430cc M    common
>> :040000 040000 5a90dc20fe7cb3f74ae1768a5b5024a902c9354d
>> a2be92ebd64fd46ad17427e8e3c12da7f10df699 M    include
>> :040000 040000 af9f333c0f56dca3a90fb7b04f385dbf39425e8d
>> 99b5f9757c78216a08220b7eb056f343658b961d M    pcbnew
>>
>>
>> Den tor. 5. jul. 2018 kl. 12.13 skrev John Beard <john.j.beard@xxxxxxxxx>:
>>>
>>> Hi,
>>>
>>> Are the qa_geometry test supposed to all work?
>>>
>>> When I run `qa_geometry`, I get 1160 errors like this:
>>>
>>> error: in "ChamferFillet/Fillet": check { chainPoints.begin(),
>>> chainPoints.end() } == { polyPoints.begin(), polyPoints.end() } has
>>> failed.
>>>
>>> Mismatch at position 0: [ 40 | 14 ] != [ 40 | 12 ]
>>> Mismatch at position 1: [ 40 | 15 ] != [ 40 | 13 ]
>>> Mismatch at position 2: [ 44 | 10 ] != [ 40 | 14 ]
>>> Mismatch at position 3: [ 44 | 18 ] != [ 40 | 15 ]
>>> Mismatch at position 4: [ 50 | 10 ] != [ 40 | 16 ]
>>> Mismatch at position 5: [ 51 | 14 ] != [ 40 | 17 ]
>>> Collections size mismatch: 6 != 25
>>>
>>> Attached is a patch that enabled CTest tests and adds qa_geometry as a
>>> test. Then you can run `make test` or `ctest` to run all tests. I
>>> think it would be good to have a single unambigous and easily
>>> understood command to be able to run unit tests?
>>>
>>> This patch explicitly excludes the "ChamferFillet/Fillet" tests as
>>> they are failing, but if those tests can be fixed, it would be good to
>>> run them too.
>>>
>>> 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


Follow ups

References