QA: Add test on GetLayerName function

This adds a test to the recent GetLayerName function, and moves
it to the base.h file for re-use for any code that needs a
layer name.

This test covers the case that caused lp:1824750 [1].

[1]: https://bugs.launchpad.net/kicad/+bug/1824750

qa/utils/kicad2step/pcb/test_base.cpp

@@ -59,11 +59,17 @@ constexpr double DegToRad( double aDeg )
 }
 
 
+class TEST_PCB_BASE_FIXTURE
+{
+public:
+    SEXPR::PARSER m_parser;
+};
+
+
 /**
  * Declare the test suite
  */
-BOOST_AUTO_TEST_SUITE( PcbBase )
-
+BOOST_FIXTURE_TEST_SUITE( PcbBase, TEST_PCB_BASE_FIXTURE )
 
 struct TEST_2D_POS_ROT
 {
@@ -98,15 +104,13 @@ BOOST_AUTO_TEST_CASE( SexprTo2DPosAndRot )
             {},
         },
         {
-            "(att 3.14 4.12 90.4)",
+            "(att 3.14 4.12 90.4)", // bad symbol name
             false,
             {},
             {},
         },
     };
 
-    SEXPR::PARSER parser;
-
     for( const auto& c : cases )
     {
         BOOST_TEST_CONTEXT( c.m_sexp )
@@ -114,7 +118,7 @@ BOOST_AUTO_TEST_CASE( SexprTo2DPosAndRot )
             DOUBLET gotPos;
             double gotRot;
 
-            std::unique_ptr<SEXPR::SEXPR> sexpr( parser.Parse( c.m_sexp ) );
+            std::unique_ptr<SEXPR::SEXPR> sexpr( m_parser.Parse( c.m_sexp ) );
 
             const bool ret = Get2DPositionAndRotation( sexpr.get(), gotPos, gotRot );
 
@@ -132,4 +136,67 @@ BOOST_AUTO_TEST_CASE( SexprTo2DPosAndRot )
     }
 }
 
+struct TEST_LAYER_NAME
+{
+    std::string m_sexp;
+    bool        m_valid;
+    std::string m_layer;
+};
+
+/**
+ * Test the layer list parser.
+ */
+BOOST_AUTO_TEST_CASE( TestGetLayerName )
+{
+    const std::vector<TEST_LAYER_NAME> cases = {
+        {
+            // Quoted string - OK
+            "(layer \"Edge.Cuts\")",
+            true,
+            "Edge.Cuts",
+        },
+        {
+            // Old KiCads exported without quotes (so, as symbols)
+            "(layer Edge.Cuts)",
+            true,
+            "Edge.Cuts",
+        },
+        {
+            // No atoms
+            "(layer)",
+            false,
+            {},
+        },
+        {
+            // Too many atoms in list
+            "(layer \"Edge.Cuts\" 2)",
+            false,
+            {},
+        },
+        {
+            // Wrong atom type
+            "(layer 2)",
+            false,
+            {},
+        },
+    };
+
+    for( const auto& c : cases )
+    {
+        BOOST_TEST_CONTEXT( c.m_sexp )
+        {
+            std::unique_ptr<SEXPR::SEXPR> sexpr( m_parser.Parse( c.m_sexp ) );
+
+            const OPT<std::string> ret = GetLayerName( *sexpr );
+
+            BOOST_CHECK_EQUAL( ret.has_value(), c.m_valid );
+
+            if( ret )
+            {
+                BOOST_CHECK_EQUAL( *ret, c.m_layer );
+            }
+        }
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()

utils/kicad2step/CMakeLists.txt

@@ -1,7 +1,3 @@
-include_directories( BEFORE
-    ${CMAKE_SOURCE_DIR}/include
-)
-
 include_directories( SYSTEM
     ${OCE_INCLUDE_DIRS}
     ${OCC_INCLUDE_DIR}
@@ -26,6 +22,7 @@ add_library( kicad2step_lib STATIC
 
 target_include_directories( kicad2step_lib PUBLIC
     ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_SOURCE_DIR}/include # for core
 )
 
 target_link_libraries( kicad2step_lib

utils/kicad2step/pcb/base.cpp

@@ -257,3 +257,27 @@ bool GetXYZRotation( SEXPR::SEXPR* data, TRIPLET& aRotation )
 
     return true;
 }
+
+
+OPT<std::string> GetLayerName( const SEXPR::SEXPR& aLayerElem )
+{
+    OPT<std::string> layer;
+
+    if( aLayerElem.GetNumberOfChildren() == 2 )
+    {
+        const auto& layerChild = *aLayerElem.GetChild( 1 );
+
+        // The layer child can be quoted (string) or unquoted (symbol)
+        // depending on PCB version.
+        if( layerChild.IsString() )
+        {
+            layer = layerChild.GetString();
+        }
+        else if( layerChild.IsSymbol() )
+        {
+            layer = layerChild.GetSymbol();
+        }
+    }
+
+    return layer;
+}

utils/kicad2step/pcb/base.h

@@ -30,6 +30,8 @@
 #ifndef KICADBASE_H
 #define KICADBASE_H
 
+#include <core/optional.h>
+
 #include <ostream>
 
 ///> Minimum distance between points to treat them as separate ones (mm)
@@ -95,4 +97,15 @@ bool Get2DCoordinate( SEXPR::SEXPR* data, DOUBLET& aCoordinate );
 bool Get3DCoordinate( SEXPR::SEXPR* data, TRIPLET& aCoordinate );
 bool GetXYZRotation( SEXPR::SEXPR* data, TRIPLET& aRotation );
 
+/**
+ * Get the layer name from a layer element, if the layer is syntactically
+ * valid.
+ *
+ * E.g. (layer "Edge.Cuts") -> "Edge.Cuts"
+ *
+ * @param  aLayerElem the s-expr element to get the name from
+ * @return            the layer name if valid, else empty
+ */
+OPT<std::string> GetLayerName( const SEXPR::SEXPR& aLayerElem );
+
 #endif  // KICADBASE_H

utils/kicad2step/pcb/kicadcurve.cpp

@@ -25,46 +25,12 @@
 
 #include <sexpr/sexpr.h>
 
-#include <core/optional.h>
-
 #include <wx/log.h>
 
 #include <iostream>
 #include <math.h>
 #include <sstream>
 
-/**
- * Get the layer name from a layer element, if the layer is syntactically
- * valid
- *
- * E.g. (layer "Edge.Cuts") -> "Edge.Cuts"
- *
- * @param  aLayerElem the s-expr element to get the name from
- * @return            the layer name if valid, else empty
- */
-static OPT<std::string> getLayerName( const SEXPR::SEXPR& aLayerElem )
-{
-    OPT<std::string> layer;
-
-    if( aLayerElem.GetNumberOfChildren() == 2 )
-    {
-        const auto& layerChild = *aLayerElem.GetChild( 1 );
-
-        // The layer child can be quoted (string) or unquoted (symbol)
-        // depending on PCB version.
-        if( layerChild.IsString() )
-        {
-            layer = layerChild.GetString();
-        }
-        else if( layerChild.IsSymbol() )
-        {
-            layer = layerChild.GetSymbol();
-        }
-    }
-
-    return layer;
-}
-
 
 KICADCURVE::KICADCURVE()
 {
@@ -152,7 +118,7 @@ bool KICADCURVE::Read( SEXPR::SEXPR* aEntry, CURVE_TYPE aCurveType )
         }
         else if( text == "layer" )
         {
-            const OPT<std::string> layer = getLayerName( *child );
+            const OPT<std::string> layer = GetLayerName( *child );
 
             if( !layer )
             {