← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] QA: Polyset Distance tests

 

Urgh, I have the inline but in another pending patch that preceded
this, so it got removed in rebase.

Sorry!

On Wed, Jan 30, 2019 at 10:27 AM Simon Richter <Simon.Richter@xxxxxxxxxx> wrote:
>
> Hi,
>
> On 30.01.19 11:21, jp charras wrote:
>
> > E:/kicad-launchpad/testing_git/qa/unit_test_utils/include/unit_test_utils/geometry.h:18:
> > multiple definition of `operator<<(std::ostream&, BOX2<VECTOR2<int> >
> > const&)'
> > CMakeFiles/qa_common_gerbview.dir/objects.a(test_shape_arc.cpp.obj):E:/kicad-launchpad/testing_git/qa/unit_test_utils/include/unit_test_utils/geometry.h:18:
> > first defined here
>
> Needs an "inline".
>
>    Simon
>
> _______________________________________________
> 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 7528b06c4e2fc0a4039c1ed10c59b861712405d8 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 +++-
 qa/unit_test_utils/include/unit_test_utils/geometry.h | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

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 ) );
         }
     }
 }
diff --git a/qa/unit_test_utils/include/unit_test_utils/geometry.h b/qa/unit_test_utils/include/unit_test_utils/geometry.h
index 2f8be6f70..3d189cc6d 100644
--- a/qa/unit_test_utils/include/unit_test_utils/geometry.h
+++ b/qa/unit_test_utils/include/unit_test_utils/geometry.h
@@ -14,7 +14,7 @@
  *
  * TODO: convert to boost_test_print_type when Boost minver > 1.64
  */
-std::ostream& operator<<( std::ostream& os, const BOX2I& aBox )
+inline std::ostream& operator<<( std::ostream& os, const BOX2I& aBox )
 {
     os << "BOX[ " << aBox.GetOrigin() << " + " << aBox.GetSize() << " ]";
     return os;
-- 
2.20.1

From 4351401b30644927f6b364e305764cd7fce7dba0 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

From eed9056a6971bc11d61629c40481bc4a40515fdd 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


Follow ups

References