kicad-developers team mailing list archive

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

HI Wayne,

Thanks. I have realised there is a test failure in qa_common_eeschema
caused by unit rounding. It's currently "masked" by the polyset
failures.

Patches attached:

1) Make polyset failures expected, so at least they won't mask other
errors or kill CI jobs while the polysets are worked on (tests log the
fail, but return 0)
2) Account for unit rounding in the Distance tests
3) A little unrelated tweak to a SHAPE_LINE_CHAIN ctor.

After these patches, I see a passing test suite again.

Cheers,

John

On Tue, Jan 29, 2019 at 11:53 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> Patches merged.
>
> Thanks,
>
> Wayne
>
> On 1/29/2019 5:40 PM, John Beard wrote:
> > Hi,
> >
> > Following the QA patch set just merged, here are a couple more.
> >
> > 1) Add tests on SHAPE_POLY_SET::Distance and a few geometry helpers
> > for concisely constructing test objects. Notably: this test would have
> > caught the bug just fixed in 90178eb68.
> > 2) Some internal QA tidy up to make the qa-internal includes clearer.
> >
> > (These tests do not concern, conflict with or affect the recent
> > SHAPE_POLY_SET Collision problems.)
> >
> > 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

From d1b7444146fc083cf5dd294d2850830c31003172 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 30 Jan 2019 09:37:15 +0000
Subject: [PATCH 2/3] QA: Account for eeschema tests unit rounding

---
 qa/common/geometry/test_shape_poly_set_distance.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qa/common/geometry/test_shape_poly_set_distance.cpp b/qa/common/geometry/test_shape_poly_set_distance.cpp
index 79de9620a..5917a9a59 100644
--- a/qa/common/geometry/test_shape_poly_set_distance.cpp
+++ b/qa/common/geometry/test_shape_poly_set_distance.cpp
@@ -31,6 +31,8 @@
 #include <qa_utils/geometry/poly_set_construction.h>
 #include <qa_utils/geometry/seg_construction.h>
 
+#include <unit_test_utils/geometry.h>
+
 /**
  * Declares the Boost test suite fixture.
  */
@@ -141,7 +143,7 @@ BOOST_AUTO_TEST_CASE( SegDistance )
             int dist = polyset.Distance( c.m_seg, c.m_seg_width );
 
             // right answer?
-            BOOST_CHECK_EQUAL( dist, c.m_exp_dist );
+            BOOST_CHECK_PREDICATE( KI_TEST::IsWithin<int>, ( dist )( c.m_exp_dist )( 1 ) );
         }
     }
 }
-- 
2.20.1

From 25c07ef88e5ebffe01ff15b95c7075971d3e6448 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 30 Jan 2019 09:12:10 +0000
Subject: [PATCH 3/3] Geom: interate ClipperLib::Path by reference

ClipperLib::Path is std::vector<IntPoint>. Iterating this with
"for( auto point : path)" could result in 'n' IntPoint copy-constructions.
It does seem GCC 8, at least, manages to optimise these constructions
out.

Replace with the "standard" for( const auto& point : path) idiom.
---
 include/geometry/shape_line_chain.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/geometry/shape_line_chain.h b/include/geometry/shape_line_chain.h
index da6dbee21..795d0012e 100644
--- a/include/geometry/shape_line_chain.h
+++ b/include/geometry/shape_line_chain.h
@@ -132,7 +132,7 @@ public:
     {
         m_points.reserve( aPath.size() );
 
-        for( auto point : aPath )
+        for( const auto& point : aPath )
             m_points.emplace_back( point.X, point.Y );
     }
 
-- 
2.20.1

From a91a016017ad30ba96d8270699183117a722ea75 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 30 Jan 2019 09:45:05 +0000
Subject: [PATCH 1/3] QA: Mark failing SHAPE_POLY_SET tests as expected
 failures

This allows the tests to remain sensitive to other errors.

When SHAPE_POLY_SET is fixed, these expected failures should be
removed.
---
 qa/common/geometry/test_shape_poly_set_collision.cpp | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/qa/common/geometry/test_shape_poly_set_collision.cpp b/qa/common/geometry/test_shape_poly_set_collision.cpp
index 828679d25..11aa8bb5d 100644
--- a/qa/common/geometry/test_shape_poly_set_collision.cpp
+++ b/qa/common/geometry/test_shape_poly_set_collision.cpp
@@ -77,7 +77,7 @@ struct CollisionFixture
 /**
  * Declares the CollisionFixture as the boost test suite fixture.
  */
-BOOST_FIXTURE_TEST_SUITE( Collision, CollisionFixture )
+BOOST_FIXTURE_TEST_SUITE( SPSCollision, CollisionFixture )
 
 /**
  * Simple dummy test to check that HasHoles() definition is right
@@ -113,11 +113,13 @@ BOOST_AUTO_TEST_CASE( PointOnEdge )
     BOOST_CHECK( !common.holeyPolySet.PointOnEdge( VECTOR2I( 200, 200 ) ) );
 }
 
+#ifdef HAVE_EXPECTED_FAILURES
+
 /**
  * This test checks that the function Contains, whose behaviour has been updated to also manage
  * holey polygons, does the right work.
  */
-BOOST_AUTO_TEST_CASE( pointInPolygonSet )
+BOOST_AUTO_TEST_CASE( pointInPolygonSet, *boost::unit_test::expected_failures( 1 ) )
 {
     // Check that the set contains the points that collide with it
     for( const VECTOR2I& point : collidingPoints )
@@ -135,7 +137,7 @@ BOOST_AUTO_TEST_CASE( pointInPolygonSet )
 /**
  * This test checks the behaviour of the Collide (with a point) method.
  */
-BOOST_AUTO_TEST_CASE( Collide )
+BOOST_AUTO_TEST_CASE( Collide, *boost::unit_test::expected_failures( 1 ) )
 {
     // When clearance = 0, the behaviour should be the same as with Contains
 
@@ -160,6 +162,8 @@ BOOST_AUTO_TEST_CASE( Collide )
     BOOST_CHECK( common.holeyPolySet.Collide( VECTOR2I( 11, 11 ), 5 ) );
 }
 
+#endif
+
 /**
  * This test checks the behaviour of the CollideVertex method, testing whether the collision with
  * vertices is well detected
-- 
2.20.1