From f6e1e24fa09b83059a70af0c33080f69db6bb507 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sun, 25 May 2025 16:55:44 -0400 Subject: [PATCH] Fix unit tests Get rid of mocking of settings, instead use actual QA test environment settings Make sure to load null project at fixture startup Improve XNODE attribute handling Fix netlist detection regex for prettified output --- common/libraries/library_manager.cpp | 15 +-- common/settings/settings_manager.cpp | 4 +- common/xnode.cpp | 113 ++++++++++-------- include/xnode.h | 6 +- pcbnew/netlist_reader/netlist_reader.cpp | 2 +- qa/data/config/9.99/kicad_common.json | 2 +- qa/data/config/9.99/sym-lib-table | 1 + qa/mocks/include/mock_pgm_base.h | 1 - qa/tests/api/test_api_module.cpp | 7 ++ qa/tests/common/test_module.cpp | 7 ++ qa/tests/eeschema/test_module.cpp | 3 + .../eeschema/test_netlist_exporter_kicad.cpp | 2 + qa/tests/eeschema/test_symbol_library.cpp | 1 + .../cadstar/test_cadstar_footprints.cpp | 6 +- qa/tests/pcbnew/test_module.cpp | 6 + qa/tests/spice/test_module.cpp | 1 + .../spice/test_netlist_exporter_spice.cpp | 8 +- qa/tests/spice/test_sim_regressions.cpp | 16 +-- 18 files changed, 119 insertions(+), 82 deletions(-) diff --git a/common/libraries/library_manager.cpp b/common/libraries/library_manager.cpp index 71108ee2fe..149deb66f8 100644 --- a/common/libraries/library_manager.cpp +++ b/common/libraries/library_manager.cpp @@ -329,20 +329,17 @@ void LIBRARY_MANAGER::LoadGlobalTables() wxCHECK( settings, /* void */ ); - wxString packagesPath; const ENV_VAR_MAP& vars = Pgm().GetLocalEnvVariables(); + std::optional packagesPath = ENV_VAR::GetVersionedEnvVarValue( vars, wxT( "3RD_PARTY" ) ); - if( std::optional v = ENV_VAR::GetVersionedEnvVarValue( vars, wxT( "3RD_PARTY" ) ) ) - packagesPath = *v; - - if( settings->m_PcmLibAutoAdd ) + if( packagesPath && settings->m_PcmLibAutoAdd ) { // Scan for libraries in PCM packages directory - wxFileName d( packagesPath, "" ); + wxFileName d( *packagesPath, "" ); if( d.DirExists() ) { - PCM_LIB_TRAVERSER traverser( packagesPath, *this, settings->m_PcmLibPrefix ); + PCM_LIB_TRAVERSER traverser( *packagesPath, *this, settings->m_PcmLibPrefix ); wxDir dir( d.GetPath() ); dir.Traverse( traverser ); @@ -369,7 +366,7 @@ void LIBRARY_MANAGER::LoadGlobalTables() [&]( const LIBRARY_TABLE_ROW& aRow ) { wxString path = GetFullURI( &aRow, true ); - return path.StartsWith( packagesPath ) && !wxFile::Exists( path ); + return path.StartsWith( *packagesPath ) && !wxFile::Exists( path ); } ); table->Rows().erase( toErase.begin(), toErase.end() ); @@ -386,7 +383,7 @@ void LIBRARY_MANAGER::LoadGlobalTables() } }; - if( settings->m_PcmLibAutoRemove ) + if( packagesPath && settings->m_PcmLibAutoRemove ) { cleanupRemovedPCMLibraries( LIBRARY_TABLE_TYPE::SYMBOL ); cleanupRemovedPCMLibraries( LIBRARY_TABLE_TYPE::FOOTPRINT ); diff --git a/common/settings/settings_manager.cpp b/common/settings/settings_manager.cpp index 7862ca9ea2..9a9ad16f9a 100644 --- a/common/settings/settings_manager.cpp +++ b/common/settings/settings_manager.cpp @@ -1045,7 +1045,9 @@ bool SETTINGS_MANAGER::LoadProject( const wxString& aFullPath, bool aSetActive ) m_projects[fullPath]->setLocalSettings( settings ); - if( aSetActive ) + // If not running from SWIG; notify the library manager of the new project + // TODO(JE) this maybe could be handled through kiway (below) in the future + if( aSetActive && PgmOrNull() ) Pgm().GetLibraryManager().ProjectChanged(); if( aSetActive && m_kiway ) diff --git a/common/xnode.cpp b/common/xnode.cpp index 0288c01668..5dfa1155b0 100644 --- a/common/xnode.cpp +++ b/common/xnode.cpp @@ -31,6 +31,58 @@ #include +XATTR::XATTR( const wxString& aName, const VALUE_TYPE& aValue ) : + wxXmlAttribute( aName, wxEmptyString ), + m_originalValue( aValue ) +{ + std::visit( [&]( T0&& arg ) + { + using T = std::decay_t; + + if constexpr( std::is_same_v ) + { + wxXmlAttribute::SetValue( wxString::Format( "%d", arg ) ); + } + else if constexpr( std::is_same_v ) + { + std::string buf; + + if( arg != 0.0 && std::fabs( arg ) <= 0.0001 ) + { + buf = fmt::format( "{:.16f}", arg ); + + // remove trailing zeros (and the decimal marker if needed) + while( !buf.empty() && buf[buf.size() - 1] == '0' ) + { + buf.pop_back(); + } + + // if the value was really small + // we may have just stripped all the zeros after the decimal + if( buf[buf.size() - 1] == '.' ) + { + buf.pop_back(); + } + } + else + { + buf = fmt::format( "{:.10g}", arg ); + } + + wxXmlAttribute::SetValue( buf ); + } + else if constexpr( std::is_same_v ) + { + wxXmlAttribute::SetValue( arg ); + } + else + { + static_assert( false, "Missing type handling in XNODE::FormatContents" ); + } + }, aValue ); +} + + void XNODE::AddBool( const wxString& aKey, bool aValue ) { AddAttribute( aKey, aValue ? wxT( "yes" ) : wxT( "no" ) ); @@ -84,65 +136,26 @@ void XNODE::FormatContents( OUTPUTFORMATTER* out ) const // output attributes first if they exist for( wxXmlAttribute* attr = GetAttributes(); attr; attr = attr->GetNext() ) { + bool quote = true; + if( auto xa = dynamic_cast( attr ) ) { XATTR::VALUE_TYPE value = xa->GetValue(); - std::visit( - [out, xa](T0&& arg) - { - using T = std::decay_t; - - if constexpr( std::is_same_v ) + std::visit( ["e]( T0&& ) { - out->Print( 0, " (%s %d)", TO_UTF8( xa->GetName() ), arg ); - } - else if constexpr( std::is_same_v ) - { - std::string buf; + using T = std::decay_t; - if( arg != 0.0 && std::fabs( arg ) <= 0.0001 ) + if constexpr( std::is_same_v || std::is_same_v ) { - buf = fmt::format( "{:.16f}", arg ); - - // remove trailing zeros (and the decimal marker if needed) - while( !buf.empty() && buf[buf.size() - 1] == '0' ) - { - buf.pop_back(); - } - - // if the value was really small - // we may have just stripped all the zeros after the decimal - if( buf[buf.size() - 1] == '.' ) - { - buf.pop_back(); - } - } - else - { - buf = fmt::format( "{:.10g}", arg ); + quote = false; } + }, value ); + } - out->Print( 0, " (%s %s)", TO_UTF8( xa->GetName() ), buf.c_str() ); - } - else if constexpr( std::is_same_v ) - { - out->Print( 0, " (%s %s)", - TO_UTF8( xa->GetName() ), - out->Quotew( arg ).c_str() ); - } - else - { - static_assert( false, "Missing type handling in XNODE::FormatContents" ); - } - }, value ); - } - else - { - out->Print( 0, " (%s %s)", - TO_UTF8( attr->GetName() ), - out->Quotew( attr->GetValue() ).c_str() ); - } + out->Print( 0, " (%s %s)", + TO_UTF8( attr->GetName() ), + quote ? out->Quotew( attr->GetValue() ).c_str() : TO_UTF8( attr->GetValue() ) ); } // we only expect to have used one of two types here: diff --git a/include/xnode.h b/include/xnode.h index 2e37a0aaf4..f885a4ff26 100644 --- a/include/xnode.h +++ b/include/xnode.h @@ -54,11 +54,7 @@ public: XATTR() {} - XATTR( const wxString& aName, const VALUE_TYPE& aValue ) : - wxXmlAttribute( aName, wxEmptyString ), - m_originalValue( aValue ) - { - } + XATTR( const wxString& aName, const VALUE_TYPE& aValue ); void SetValue( const VALUE_TYPE& aValue ) { m_originalValue = aValue; } diff --git a/pcbnew/netlist_reader/netlist_reader.cpp b/pcbnew/netlist_reader/netlist_reader.cpp index e468db3050..636da335e9 100644 --- a/pcbnew/netlist_reader/netlist_reader.cpp +++ b/pcbnew/netlist_reader/netlist_reader.cpp @@ -58,7 +58,7 @@ NETLIST_READER::NETLIST_FILE_T NETLIST_READER::GuessNetlistFileType( LINE_READER wxASSERT( reLegacy.IsValid() ); // Our new netlist format starts by "(export (version " - wxRegEx reKicad( wxT( "[ ]*\\(export[ ]+" ), wxRE_ADVANCED ); + wxRegEx reKicad( wxT( "[ ]*\\(export(\\s+\\(version)?" ), wxRE_ADVANCED ); wxASSERT( reKicad.IsValid() ); wxString line; diff --git a/qa/data/config/9.99/kicad_common.json b/qa/data/config/9.99/kicad_common.json index 5ac4d67b38..8c7494096b 100644 --- a/qa/data/config/9.99/kicad_common.json +++ b/qa/data/config/9.99/kicad_common.json @@ -74,7 +74,7 @@ "version": 4 }, "netclass_panel": { - "eeschema_shown_columns": "0 11 12 13 14", + "eeschema_shown_columns": "0 19 20 21 22", "pcbnew_shown_columns": "0 1 2 3 4 5 6 7 8 9", "sash_pos": 160 }, diff --git a/qa/data/config/9.99/sym-lib-table b/qa/data/config/9.99/sym-lib-table index 55e8b41d28..4e797afa67 100644 --- a/qa/data/config/9.99/sym-lib-table +++ b/qa/data/config/9.99/sym-lib-table @@ -2,4 +2,5 @@ (version 7) (lib (name "Device")(type "KiCad")(uri "${KICAD9_SYMBOL_DIR}/Device.kicad_sym")(options "")(descr "Generic symbols for common devices")) (lib (name "power")(type "KiCad")(uri "${KICAD9_SYMBOL_DIR}/power.kicad_sym")(options "")(descr "Power symbols, special power flags")) + (lib (name "future")(type "Unknown")(uri "${KICAD_CONFIG_HOME}/../libraries/unknown.kicad_sym")(options "")(descr "An unknown type of library")) ) diff --git a/qa/mocks/include/mock_pgm_base.h b/qa/mocks/include/mock_pgm_base.h index 3ee2b495b9..5d9a74e2ea 100644 --- a/qa/mocks/include/mock_pgm_base.h +++ b/qa/mocks/include/mock_pgm_base.h @@ -54,7 +54,6 @@ MOCK_BASE_CLASS( MOCK_PGM_BASE, PGM_BASE ) MOCK_METHOD( WritePdfBrowserInfos, 0, void() ); MOCK_METHOD( SetLocalEnvVariable, 2, bool( const wxString&, const wxString& ) ); MOCK_METHOD( SetLocalEnvVariables, 0, void() ); - MOCK_CONST_METHOD( GetLocalEnvVariables, 0, ENV_VAR_MAP&() ); int GetSelectedLanguageIdentifier() const override { diff --git a/qa/tests/api/test_api_module.cpp b/qa/tests/api/test_api_module.cpp index 39690f9088..3925a9825a 100644 --- a/qa/tests/api/test_api_module.cpp +++ b/qa/tests/api/test_api_module.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -45,7 +46,13 @@ bool init_unit_test() wxSetAssertHandler( &KI_TEST::wxAssertThrower ); Pgm().InitPgm( true, true, true ); + const MOCK_PGM_BASE& program = static_cast( Pgm() ); + + MOCK_EXPECT( program.GetCommonSettings ) + .returns( Pgm().GetSettingsManager().GetCommonSettings() ); + + Pgm().GetSettingsManager().LoadProject( "" ); } return ok; diff --git a/qa/tests/common/test_module.cpp b/qa/tests/common/test_module.cpp index ed8ea555bb..25ef07b690 100644 --- a/qa/tests/common/test_module.cpp +++ b/qa/tests/common/test_module.cpp @@ -26,6 +26,8 @@ */ #include #include +#include +#include #include #include @@ -44,7 +46,12 @@ bool init_unit_test() bool ok = wxInitialize(); if( ok ) + { Pgm().InitPgm( true, true, true ); + Pgm().GetSettingsManager().RegisterSettings( new KICAD_SETTINGS, false ); + Pgm().GetSettingsManager().Load(); + Pgm().GetSettingsManager().LoadProject( "" ); + } return ok; } diff --git a/qa/tests/eeschema/test_module.cpp b/qa/tests/eeschema/test_module.cpp index 8741b9a55e..9e3969dc8a 100644 --- a/qa/tests/eeschema/test_module.cpp +++ b/qa/tests/eeschema/test_module.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -55,9 +56,11 @@ bool init_unit_test() wxSetAssertHandler( &KI_TEST::wxAssertThrower ); Pgm().InitPgm( true, true, true ); + Pgm().GetSettingsManager().RegisterSettings( new KICAD_SETTINGS, false ); Pgm().GetSettingsManager().RegisterSettings( new EESCHEMA_SETTINGS, false ); Pgm().GetSettingsManager().RegisterSettings( new SYMBOL_EDITOR_SETTINGS, false ); Pgm().GetSettingsManager().Load(); + Pgm().GetSettingsManager().LoadProject( "" ); return ok; } diff --git a/qa/tests/eeschema/test_netlist_exporter_kicad.cpp b/qa/tests/eeschema/test_netlist_exporter_kicad.cpp index ab5364d64a..54f967a748 100644 --- a/qa/tests/eeschema/test_netlist_exporter_kicad.cpp +++ b/qa/tests/eeschema/test_netlist_exporter_kicad.cpp @@ -33,6 +33,7 @@ public: std::unique_ptr netlistReader( NETLIST_READER::GetNetlistReader( &golden, GetNetlistPath(), wxEmptyString ) ); + BOOST_ASSERT( netlistReader ); BOOST_REQUIRE_NO_THROW( netlistReader->LoadNetlist() ); } @@ -40,6 +41,7 @@ public: std::unique_ptr netlistReader( NETLIST_READER::GetNetlistReader( &test, GetNetlistPath( true ), wxEmptyString ) ); + BOOST_ASSERT( netlistReader ); BOOST_REQUIRE_NO_THROW( netlistReader->LoadNetlist() ); } diff --git a/qa/tests/eeschema/test_symbol_library.cpp b/qa/tests/eeschema/test_symbol_library.cpp index 404695f635..920eb30b5d 100644 --- a/qa/tests/eeschema/test_symbol_library.cpp +++ b/qa/tests/eeschema/test_symbol_library.cpp @@ -68,6 +68,7 @@ BOOST_AUTO_TEST_CASE( ProjectLibraryTable ) BOOST_REQUIRE( rows[0]->URI() == "${KIPRJMOD}/Device.kicad_sym" ); SYMBOL_LIBRARY_MANAGER_ADAPTER adapter( manager ); + adapter.LoadOne( "Device" ); std::vector symbols = adapter.GetSymbols( "Device" ); diff --git a/qa/tests/pcbnew/pcb_io/cadstar/test_cadstar_footprints.cpp b/qa/tests/pcbnew/pcb_io/cadstar/test_cadstar_footprints.cpp index 39ef7cdd76..96ae1f6433 100644 --- a/qa/tests/pcbnew/pcb_io/cadstar/test_cadstar_footprints.cpp +++ b/qa/tests/pcbnew/pcb_io/cadstar/test_cadstar_footprints.cpp @@ -68,8 +68,10 @@ BOOST_AUTO_TEST_CASE( CadstarFootprintImport ) wxArrayString cstarFootprintNames; wxArrayString kicadFootprintNames; - cstarPlugin.FootprintEnumerate( cstarFootprintNames, cstarLibraryPath, true, nullptr ); - kicadPlugin.FootprintEnumerate( kicadFootprintNames, kicadLibraryPath, true, nullptr ); + BOOST_REQUIRE_NO_THROW( + cstarPlugin.FootprintEnumerate( cstarFootprintNames, cstarLibraryPath, true, nullptr ) ); + BOOST_REQUIRE_NO_THROW( + kicadPlugin.FootprintEnumerate( kicadFootprintNames, kicadLibraryPath, true, nullptr ) ); BOOST_CHECK_EQUAL( cstarFootprintNames.GetCount(), kicadFootprintNames.GetCount() ); diff --git a/qa/tests/pcbnew/test_module.cpp b/qa/tests/pcbnew/test_module.cpp index 2d2cc77b8b..5d105b0716 100644 --- a/qa/tests/pcbnew/test_module.cpp +++ b/qa/tests/pcbnew/test_module.cpp @@ -57,7 +57,13 @@ bool init_unit_test() Pgm().InitPgm( true, true, true ); Pgm().GetSettingsManager().RegisterSettings( new PCBNEW_SETTINGS, false ); Pgm().GetSettingsManager().Load(); + const MOCK_PGM_BASE& program = static_cast( Pgm() ); + + MOCK_EXPECT( program.GetCommonSettings ) + .returns( Pgm().GetSettingsManager().GetCommonSettings() ); + + Pgm().GetSettingsManager().LoadProject( "" ); } return ok; diff --git a/qa/tests/spice/test_module.cpp b/qa/tests/spice/test_module.cpp index a5265cfb9d..ddb42f6276 100644 --- a/qa/tests/spice/test_module.cpp +++ b/qa/tests/spice/test_module.cpp @@ -57,6 +57,7 @@ bool init_unit_test() Pgm().GetSettingsManager().RegisterSettings( new EESCHEMA_SETTINGS, false ); Pgm().GetSettingsManager().RegisterSettings( new SYMBOL_EDITOR_SETTINGS, false ); Pgm().GetSettingsManager().Load(); + Pgm().GetSettingsManager().LoadProject( "" ); return ok; } diff --git a/qa/tests/spice/test_netlist_exporter_spice.cpp b/qa/tests/spice/test_netlist_exporter_spice.cpp index 8870d49281..a91f8e0181 100644 --- a/qa/tests/spice/test_netlist_exporter_spice.cpp +++ b/qa/tests/spice/test_netlist_exporter_spice.cpp @@ -37,8 +37,8 @@ BOOST_AUTO_TEST_CASE( Rectifier ) { LOCALE_IO dummy; - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "rectifier" ); TestTranPoint( 0, { { "V(/in)", 0 }, { "V(/out)", 0 } } ); @@ -61,8 +61,8 @@ BOOST_AUTO_TEST_CASE( Opamp ) // Instead of Simulation_SPICE:OPAMP, we use Amplifier_Operational:MCP6001-OT because its pins // are not ordered by pin numbers, which is a possible failure condition. - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "opamp" ); TestTranPoint( 0, { { "V(/in)", 0 }, { "V(/out)", 0 } } ); diff --git a/qa/tests/spice/test_sim_regressions.cpp b/qa/tests/spice/test_sim_regressions.cpp index 1062724423..c51798c6d7 100644 --- a/qa/tests/spice/test_sim_regressions.cpp +++ b/qa/tests/spice/test_sim_regressions.cpp @@ -91,8 +91,8 @@ BOOST_FIXTURE_TEST_CASE( WindowsPaths, TEST_SIM_REGRESSIONS_FIXTURE ) { LOCALE_IO dummy; - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "issue13591" ); TestTranPoint( 100e-6, { { "I(R1)", 0 }, { "I(R2)", 0 } }, 0.00001 ); @@ -133,8 +133,8 @@ BOOST_FIXTURE_TEST_CASE( DualNMOSAmp, TEST_SIM_REGRESSIONS_FIXTURE ) { LOCALE_IO dummy; - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "issue13162" ); TestTranPoint( 0.030, { { "V(out)", 0.000829682 } } ); @@ -146,8 +146,8 @@ BOOST_FIXTURE_TEST_CASE( VariableSubstitutions, TEST_SIM_REGRESSIONS_FIXTURE ) { LOCALE_IO dummy; - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "issue12505" ); TestTranPoint( 0.015, { { "V(Net-_R1-Pad2_)", -311 } } ); @@ -159,8 +159,8 @@ BOOST_FIXTURE_TEST_CASE( IBISSim, TEST_SIM_REGRESSIONS_FIXTURE ) { LOCALE_IO dummy; - const MOCK_PGM_BASE& program = static_cast( Pgm() ); - MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); + // const MOCK_PGM_BASE& program = static_cast( Pgm() ); + // MOCK_EXPECT( program.GetLocalEnvVariables ).returns( ENV_VAR_MAP() ); TestNetlist( "issue16223" ); TestTranPoint( 0.0, { { "V(PRBS_OUTPUT)", 5.114 } } );