QA: add a library-mode footprint load/test/save test

This is not only a useful place to hang regression tests,
but also allows to catch defects specific to loading or
saving footprints.

For example, it would catch:

Relates-To: https://gitlab.com/kicad/code/kicad/-/issues/19713

qa/data/pcbnew/plugins/kicad_sexpr/fp.pretty/R_0201_0603Metric.kicad_mod

@@ -0,0 +1,197 @@
+(footprint "R_0201_0603Metric"
+	(version 20240108)
+	(generator "pcbnew")
+	(generator_version "8.0")
+	(layer "F.Cu")
+	(descr "Resistor SMD 0201 (0603 Metric), square (rectangular) end terminal, IPC_7351 nominal, (Body size source: https://www.vishay.com/docs/20052/crcw0201e3.pdf), generated with kicad-footprint-generator")
+	(tags "resistor")
+	(property "Reference" "REF**"
+		(at 0 -1.05 0)
+		(layer "F.SilkS")
+		(uuid "e45e00c2-1c2b-4f80-9271-8f80d81b2a70")
+		(effects
+			(font
+				(size 1 1)
+				(thickness 0.15)
+			)
+		)
+	)
+	(property "Value" "R_0201_0603Metric"
+		(at 0 1.05 0)
+		(layer "F.Fab")
+		(uuid "3ed3ef8f-c99a-41fc-9a16-221a6bfc2a16")
+		(effects
+			(font
+				(size 1 1)
+				(thickness 0.15)
+			)
+		)
+	)
+	(property "Footprint" ""
+		(at 0 0 0)
+		(unlocked yes)
+		(layer "F.Fab")
+		(hide yes)
+		(uuid "a4e2a71f-8d2b-47ca-93c1-367dee63d8b5")
+		(effects
+			(font
+				(size 1.27 1.27)
+			)
+		)
+	)
+	(property "Datasheet" ""
+		(at 0 0 0)
+		(unlocked yes)
+		(layer "F.Fab")
+		(hide yes)
+		(uuid "eda170bb-c944-4327-9a25-6d9f824f5968")
+		(effects
+			(font
+				(size 1.27 1.27)
+			)
+		)
+	)
+	(property "Description" ""
+		(at 0 0 0)
+		(unlocked yes)
+		(layer "F.Fab")
+		(hide yes)
+		(uuid "65eea031-3ae9-41ad-8f9f-6ef455085ce0")
+		(effects
+			(font
+				(size 1.27 1.27)
+			)
+		)
+	)
+	(attr smd)
+	(fp_line
+		(start -0.7 -0.35)
+		(end 0.7 -0.35)
+		(stroke
+			(width 0.05)
+			(type solid)
+		)
+		(layer "F.CrtYd")
+		(uuid "cd6f3c00-9c19-46c7-8b63-296ba8c3e3d0")
+	)
+	(fp_line
+		(start -0.7 0.35)
+		(end -0.7 -0.35)
+		(stroke
+			(width 0.05)
+			(type solid)
+		)
+		(layer "F.CrtYd")
+		(uuid "021628bd-92eb-4251-b890-352b8e05add9")
+	)
+	(fp_line
+		(start 0.7 -0.35)
+		(end 0.7 0.35)
+		(stroke
+			(width 0.05)
+			(type solid)
+		)
+		(layer "F.CrtYd")
+		(uuid "8ff9ffbd-6367-42cd-981b-1c93eb62a182")
+	)
+	(fp_line
+		(start 0.7 0.35)
+		(end -0.7 0.35)
+		(stroke
+			(width 0.05)
+			(type solid)
+		)
+		(layer "F.CrtYd")
+		(uuid "efdbce53-b1d1-43b8-9d32-2efb7d771ccd")
+	)
+	(fp_line
+		(start -0.3 -0.15)
+		(end 0.3 -0.15)
+		(stroke
+			(width 0.1)
+			(type solid)
+		)
+		(layer "F.Fab")
+		(uuid "e4dde74c-0933-44de-b588-4c0fde20c9f5")
+	)
+	(fp_line
+		(start -0.3 0.15)
+		(end -0.3 -0.15)
+		(stroke
+			(width 0.1)
+			(type solid)
+		)
+		(layer "F.Fab")
+		(uuid "2a65fd0d-efdf-4159-b35a-41e435728c4a")
+	)
+	(fp_line
+		(start 0.3 -0.15)
+		(end 0.3 0.15)
+		(stroke
+			(width 0.1)
+			(type solid)
+		)
+		(layer "F.Fab")
+		(uuid "8f895502-ab9f-43c3-a105-6662a16418d4")
+	)
+	(fp_line
+		(start 0.3 0.15)
+		(end -0.3 0.15)
+		(stroke
+			(width 0.1)
+			(type solid)
+		)
+		(layer "F.Fab")
+		(uuid "fd9eedea-9b5c-4077-bb85-c38eca0b131d")
+	)
+	(fp_text user "${REFERENCE}"
+		(at 0 -0.68 0)
+		(layer "F.Fab")
+		(uuid "f988840e-b08d-4b6e-9589-79b3a7bf0968")
+		(effects
+			(font
+				(size 0.25 0.25)
+				(thickness 0.04)
+			)
+		)
+	)
+	(pad "" smd roundrect
+		(at -0.345 0)
+		(size 0.318 0.36)
+		(layers "F.Paste")
+		(roundrect_rratio 0.25)
+		(uuid "a4ae16a7-8a3a-4116-a03a-a033592a0aba")
+	)
+	(pad "" smd roundrect
+		(at 0.345 0)
+		(size 0.318 0.36)
+		(layers "F.Paste")
+		(roundrect_rratio 0.25)
+		(uuid "bf7d15b9-64d8-4447-80bc-cf2dc469d35e")
+	)
+	(pad "1" smd roundrect
+		(at -0.32 0)
+		(size 0.46 0.4)
+		(layers "F.Cu" "F.Mask")
+		(roundrect_rratio 0.25)
+		(uuid "5b63156c-3c32-4c95-bebb-6d18efe0eed7")
+	)
+	(pad "2" smd roundrect
+		(at 0.32 0)
+		(size 0.46 0.4)
+		(layers "F.Cu" "F.Mask")
+		(roundrect_rratio 0.25)
+		(uuid "a66fc88c-2b90-4f27-a1c9-aa8c8902aa77")
+	)
+	(model "${KICAD8_3DMODEL_DIR}/Resistor_SMD.3dshapes/R_0201_0603Metric.wrl"
+		(offset
+			(xyz 0 0 0)
+		)
+		(scale
+			(xyz 1 1 1)
+		)
+		(rotate
+			(xyz 0 0 0)
+		)
+	)
+)

qa/pcbnew_utils/board_file_utils.cpp

@@ -146,5 +146,11 @@ std::unique_ptr<FOOTPRINT> ReadFootprintFromFileOrStream( const std::string& aFi
 }
 
 
+void DumpFootprintToFile( const FOOTPRINT& aFootprint, const std::string& aLibraryPath )
+{
+    PCB_IO_KICAD_SEXPR io;
+    io.FootprintSave( aLibraryPath, &aFootprint, nullptr );
+}
+
 
 } // namespace KI_TEST

qa/pcbnew_utils/board_test_utils.cpp

@@ -127,9 +127,48 @@ BOARD_ITEM& RequireBoardItemWithTypeAndId( const BOARD& aBoard, KICAD_T aItemTyp
 }
 
 
+/**
+ * A temporary directory that will be deleted when it goes out of scope.
+ */
+class TEMPORARY_DIRECTORY
+{
+public:
+    /**
+     * Create a temporary directory with a given prefix and suffix. The directory will be
+     * created in the system temporary directory, and will not be pre-existing.
+     */
+    TEMPORARY_DIRECTORY( const std::string& aNamePrefix, const std::string aSuffix )
+    {
+        int i = 0;
+
+        // Find a unique directory name
+        while( true )
+        {
+            m_path = std::filesystem::temp_directory_path()
+                     / ( aNamePrefix + std::to_string( i ) + aSuffix );
+
+            if( !std::filesystem::exists( m_path ) )
+                break;
+
+            i++;
+        }
+
+        wxASSERT( !std::filesystem::exists( m_path ) );
+        std::filesystem::create_directories( m_path );
+    }
+
+    ~TEMPORARY_DIRECTORY() { std::filesystem::remove_all( m_path ); }
+
+    const std::filesystem::path& GetPath() const { return m_path; }
+
+private:
+    std::filesystem::path m_path;
+};
+
+
 void LoadAndTestBoardFile( const wxString aRelativePath, bool aRoundtrip,
                            std::function<void( BOARD& )> aBoardTestFunction,
-                           std::optional<int> aExpectedBoardVersion )
+                           std::optional<int>            aExpectedBoardVersion )
 {
     const std::string absBoardPath =
             KI_TEST::GetPcbnewTestDataDir() + aRelativePath.ToStdString() + ".kicad_pcb";
@@ -152,8 +191,9 @@ void LoadAndTestBoardFile( const wxString aRelativePath, bool aRoundtrip,
 
     if( aRoundtrip )
     {
-        const auto savePath = std::filesystem::temp_directory_path()
-                              / ( aRelativePath.ToStdString() + ".kicad_pcb" );
+        TEMPORARY_DIRECTORY tempLib( "kicad_qa_brd_roundtrip", "" );
+
+        const auto savePath = tempLib.GetPath() / ( aRelativePath.ToStdString() + ".kicad_pcb" );
         KI_TEST::DumpBoardToFile( *board1, savePath.string() );
 
         std::unique_ptr<BOARD> board2 = KI_TEST::ReadBoardFromFileOrStream( savePath.string() );
@@ -167,6 +207,60 @@ void LoadAndTestBoardFile( const wxString aRelativePath, bool aRoundtrip,
 }
 
 
+void LoadAndTestFootprintFile( const wxString& aLibRelativePath, const wxString& aFpName,
+                               bool                              aRoundtrip,
+                               std::function<void( FOOTPRINT& )> aFootprintTestFunction,
+                               std::optional<int>                aExpectedFootprintVersion )
+{
+    const std::string absFootprintPath = KI_TEST::GetPcbnewTestDataDir()
+                                         + aLibRelativePath.ToStdString() + "/"
+                                         + aFpName.ToStdString() + ".kicad_mod";
+
+    BOOST_TEST_MESSAGE( "Loading footprint to test: " << absFootprintPath );
+    std::unique_ptr<FOOTPRINT> fp1 = KI_TEST::ReadFootprintFromFileOrStream( absFootprintPath );
+
+    // Should load - if it doesn't we're done for
+    BOOST_REQUIRE( fp1 );
+
+    BOOST_TEST_MESSAGE( "Testing loaded footprint (value: " << fp1->GetValue() << ")" );
+    aFootprintTestFunction( *fp1 );
+
+    // If we care about the board version, check it now - but not after a roundtrip
+    // (as the version will be updated to the current version)
+    if( aExpectedFootprintVersion )
+    {
+        BOOST_CHECK_EQUAL( fp1->GetFileFormatVersionAtLoad(), *aExpectedFootprintVersion );
+    }
+
+    if( aRoundtrip )
+    {
+        /**
+         * Use a temporary directory, so that if something goes wrong and the file isn't
+         * written properly, we don't leave a library behind that will cause exceptions
+         * when the cache is set up on future runs.
+         */
+        TEMPORARY_DIRECTORY tempLib( "kicad_qa_fp_roundtrip", ".pretty" );
+        const wxString      fpFilename = fp1->GetFPID().GetLibItemName() + ".kicad_mod";
+
+        BOOST_TEST_MESSAGE( "Resaving footprint: " << fpFilename << " in " << tempLib.GetPath() );
+
+        KI_TEST::DumpFootprintToFile( *fp1, tempLib.GetPath().string() );
+
+        const auto fp2Path = tempLib.GetPath() / fpFilename.ToStdString();
+
+        BOOST_TEST_MESSAGE( "Re-reading footprint: " << fpFilename << " in " << tempLib.GetPath() );
+
+        std::unique_ptr<FOOTPRINT> fp2 = KI_TEST::ReadFootprintFromFileOrStream( fp2Path );
+
+        // Should load again
+        BOOST_REQUIRE( fp2 );
+
+        BOOST_TEST_MESSAGE( "Testing roundtripped (saved/reloaded) file" );
+        aFootprintTestFunction( *fp2 );
+    }
+}
+
+
 void FillZones( BOARD* m_board )
 {
     TOOL_MANAGER toolMgr;

qa/pcbnew_utils/include/pcbnew_utils/board_file_utils.h

@@ -58,6 +58,11 @@ std::string GetPcbnewTestDataDir();
  */
 void DumpBoardToFile( BOARD& aBoard, const std::string& aFilename );
 
+/**
+ * Same as DumpBoardToFile, but for footprints
+ */
+void DumpFootprintToFile( const FOOTPRINT& aFootprint, const std::string& aLibraryPath );
+
 /**
  * Utility function to read a #BOARD_ITEM (probably a #FOOTPRINT or a #BOARD)
  * from a file.

qa/pcbnew_utils/include/pcbnew_utils/board_test_utils.h

@@ -206,6 +206,15 @@ void LoadAndTestBoardFile( const wxString aRelativePath, bool aRoundtrip,
                            std::function<void( BOARD& )> aBoardTestFunction,
                            std::optional<int> aExpectedBoardVersion = std::nullopt );
 
+/**
+ * Same as LoadAndTestBoardFile, but for footprints
+ */
+void LoadAndTestFootprintFile( const wxString& aLibRelativePath, const wxString& aFpName,
+                               bool                              aRoundtrip,
+                               std::function<void( FOOTPRINT& )> aFootprintTestFunction,
+                               std::optional<int>                aExpectedFootprintVersion );
+
+
 void FillZones( BOARD* m_board );
 
 

qa/tests/pcbnew/CMakeLists.txt

@@ -38,6 +38,7 @@ set( QA_PCBNEW_SRCS
     test_graphics_import_mgr.cpp
     test_group_load_save.cpp
     test_footprint_load_save.cpp
+    test_fp_lib_load_save.cpp
     test_io_mgr.cpp
     test_lset.cpp
     test_pns_basics.cpp

qa/tests/pcbnew/test_footprint_load_save.cpp

@@ -32,12 +32,6 @@
 namespace
 {
 
-struct FOOTPRINT_LOAD_TEST_FIXTURE
-{
-    FOOTPRINT_LOAD_TEST_FIXTURE() {}
-};
-
-
 struct FOOTPRINT_LOAD_TEST_CASE
 {
     // Which footprint to look at in the file
@@ -53,12 +47,18 @@ struct FOOTPRINT_LOAD_BOARD_TEST_CASE
     // The board to load
     wxString m_boardFileRelativePath;
     // List of images to check
-    std::vector<FOOTPRINT_LOAD_TEST_CASE> m_imageCases;
+    std::vector<FOOTPRINT_LOAD_TEST_CASE> m_fpCases;
 };
 
 } // namespace
 
-BOOST_FIXTURE_TEST_CASE( FootprintLoadSave, FOOTPRINT_LOAD_TEST_FIXTURE )
+
+/**
+ * Simple tests cases that load a board file and check expected properties of the footprints.
+ * This is not the same as FpLibLoadSave as it loads _board files_, and not footprint
+ * files from a library.
+ */
+BOOST_AUTO_TEST_CASE( FootprintLoadSave )
 {
     const std::vector<FOOTPRINT_LOAD_BOARD_TEST_CASE> testCases{
         {
@@ -83,16 +83,16 @@ BOOST_FIXTURE_TEST_CASE( FootprintLoadSave, FOOTPRINT_LOAD_TEST_FIXTURE )
     {
         const auto doBoardTest = [&]( const BOARD& aBoard )
         {
-            for( const FOOTPRINT_LOAD_TEST_CASE& fooprintTestCase : testCase.m_imageCases )
+            for( const FOOTPRINT_LOAD_TEST_CASE& fooprintTestCase : testCase.m_fpCases )
             {
                 BOOST_TEST_MESSAGE( "Checking for footprint with UUID: "
                                     << fooprintTestCase.m_footprintUuid.AsString() );
 
-                const auto& image = static_cast<FOOTPRINT&>( KI_TEST::RequireBoardItemWithTypeAndId(
+                const auto& fp = static_cast<FOOTPRINT&>( KI_TEST::RequireBoardItemWithTypeAndId(
                         aBoard, PCB_FOOTPRINT_T, fooprintTestCase.m_footprintUuid ) );
 
-                BOOST_CHECK_EQUAL( image.IsLocked(), fooprintTestCase.m_expectedLocked );
-                BOOST_CHECK_EQUAL( image.GetPosition(), fooprintTestCase.m_expectedPos * 1000000 );
+                BOOST_CHECK_EQUAL( fp.IsLocked(), fooprintTestCase.m_expectedLocked );
+                BOOST_CHECK_EQUAL( fp.GetPosition(), fooprintTestCase.m_expectedPos * 1000000 );
             }
         };
 

qa/tests/pcbnew/test_fp_lib_load_save.cpp

@@ -0,0 +1,81 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright The KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <board.h>
+#include <kiid.h>
+#include <footprint.h>
+#include <pcbnew_utils/board_file_utils.h>
+#include <pcbnew_utils/board_test_utils.h>
+#include <qa_utils/wx_utils/unit_test_utils.h>
+#include <settings/settings_manager.h>
+
+namespace
+{
+
+struct FPLIB_LOAD_FP_TEST_CASE
+{
+    // The FP to load
+    wxString m_libraryPath;
+    wxString m_fpName;
+
+    std::optional<unsigned> m_expectedFootprintVersion;
+
+    // If set, the expected number of pads in the footprint
+    std::optional<unsigned> m_expectedPadCount;
+};
+
+} // namespace
+
+/**
+ * Simple tests cases that run though the given FPs and checks some simple properties.
+ *
+ * This is not the same as FootprintLoadSave, as that tests the loading and saving of
+ * _board files_ that contain footprints. This tests loading and saving of _footprint
+ * files_, and includes at least some of the library IO code.
+ */
+BOOST_AUTO_TEST_CASE( FpLibLoadSave )
+{
+    const std::vector<FPLIB_LOAD_FP_TEST_CASE> testCases{
+        {
+                "plugins/kicad_sexpr/fp.pretty",
+                "R_0201_0603Metric",
+                20240108U,
+                // 2 SMD pads, 2 paste pads
+                4U,
+        },
+    };
+
+    for( const FPLIB_LOAD_FP_TEST_CASE& testCase : testCases )
+    {
+        const auto doFootprintTest = [&]( const FOOTPRINT& aBoard )
+        {
+            if( testCase.m_expectedPadCount )
+            {
+                BOOST_CHECK_EQUAL( aBoard.Pads().size(), *testCase.m_expectedPadCount );
+            }
+        };
+
+        KI_TEST::LoadAndTestFootprintFile( testCase.m_libraryPath, testCase.m_fpName, true,
+                                           doFootprintTest, testCase.m_expectedFootprintVersion );
+    }
+}