kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

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