NREL / ssc

SAM Simulation Core (SSC) contains the underlying performance and financial models for SAM
BSD 3-Clause "New" or "Revised" License
78 stars 83 forks source link

Update tests to latest version of gtest #806

Open brtietz opened 2 years ago

brtietz commented 2 years ago

In https://github.com/NREL/ssc/runs/6181437239?check_suite_focus=true, the tests failed due to an incompatibility with the latest version of gtest with the following error:

In file included from /Users/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_tcstrough_empirical_test.cpp:23: In file included from /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest.h:60: In file included from /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-death-test.h:43: In file included from /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/internal/gtest-death-test-internal.h:46: In file included from /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-matchers.h:48: In file included from /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-printers.h:114: /Users/runner/work/ssc/ssc/googletest/googletest/include/gtest/internal/gtest-internal.h:477:40: error: cannot initialize return object of type 'testing::Test ' with an rvalue of type 'csp_trough_EmpiricalTroughCmod_Phoenix_NoFinancial_Test ' Test* CreateTest() override { return new TestClass; } ^~~ /Users/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_tcstrough_empirical_test.cpp:233:1: note: in instantiation of member function 'testing::internal::TestFactoryImpl::CreateTest' requested here NAMESPACE_TEST(csp_trough, EmpiricalTroughCmod, Phoenix_NoFinancial) ^ /Users/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:45:3: note: expanded from macro 'NAMESPACE_TEST' NAMESPACE_GTESTTEST(namespace_name, test_case_name, test_name,\ ^ /Users/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:38:13: note: expanded from macro 'NAMESPACE_GTESTTEST' new ::testing::internal::TestFactoryImpl<\ ^ 16 errors generated.

The short term fix was to pin the version of gtest at the last version that worked: https://github.com/NREL/ssc/pull/804

This issue covers the long term fix: rewriting our tests to remove the errors.

dguittet commented 6 months ago

This is resolved in #1035

tyneises commented 6 months ago

@dguittet can we update Step 4 in the build instructions? https://github.com/NREL/SAM/wiki/Windows-Build-Instructions

brtietz commented 6 months ago

That's tricky - 1035 is going to go into develop for the 2024 release, but the issue will still exist on patch (current develop). Is there a way to cherry-pick the fix to the current branch?

dguittet commented 6 months ago

I think it'll be possible to isolate the fix for patch

dguittet commented 6 months ago

@tyneises I don't think we can change the googletest version until this has been merged though