Fix error so latest version of googletest can be used

[Matthew-Boyd]

Fix error in googletest macro so that the latest version of googletest can be used.

@brtietz I think the installation instructions can be updated now to use the latest gtest version

Closes #806

[Matthew-Boyd]

@brtietz I think the errors here in the CI are because the older version of gtest is still being used, while these new changes are meant for compatibility with the latest version.

[brtietz]

I was able to build successfully on Windows. The errors on Mac and Linux look different now. Is it better to remove the copy & assign macro or update these additional tests?

In file included from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-message.h:55:0,
                 from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-assertion-result.h:46,
                 from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest.h:59,
                 from /home/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_etes_etes_test.cpp:34:
/home/runner/work/ssc/ssc/googletest/googletest/include/gtest/internal/gtest-port.h:696:23: error: return type specification for constructor invalid
   type(type const&) = delete;                 \
                       ^
/home/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:39:7: note: in expansion of macro ‘GTEST_DISALLOW_COPY_AND_ASSIGN_’
   int GTEST_DISALLOW_COPY_AND_ASSIGN_(\
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:57:3: note: in expansion of macro ‘NAMESPACE_GTEST_TEST_’
   NAMESPACE_GTEST_TEST_(namespace_name, test_case_name, test_name,\
   ^~~~~~~~~~~~~~~~~~~~~
/home/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_etes_etes_test.cpp:43:1: note: in expansion of macro ‘NAMESPACE_TEST’
 NAMESPACE_TEST(etes_etes_test, EtesEtesCmod, Default_NoFinancial)
 ^~~~~~~~~~~~~~
[ 83%] Building CXX object test/CMakeFiles/Test.dir/ssc_test/cmod_fuelcell_test.cpp.o
cd /home/runner/work/ssc/ssc/ssc/build/test && /usr/bin/g++  -I/home/runner/work/ssc/ssc/googletest/include -I/home/runner/work/ssc/ssc/googletest/googletest/include -I/home/runner/work/ssc/ssc/googletest/googletest/include/gtest -I/home/runner/work/ssc/ssc/ssc/test/. -I/home/runner/work/ssc/ssc/ssc/test/input_cases -I/home/runner/work/ssc/ssc/ssc/test/shared_test -I/home/runner/work/ssc/ssc/ssc/test/ssc_test -I/home/runner/work/ssc/ssc/ssc/test/tcs_test -I/home/runner/work/ssc/ssc/ssc/test/../ssc -I/home/runner/work/ssc/ssc/ssc/test/../tcs -I/home/runner/work/ssc/ssc/ssc/test/../solarpilot -I/home/runner/work/ssc/ssc/ssc/test/../shared -I/home/runner/work/ssc/ssc/ssc/test/../splinter -O3 -DNDEBUG -fPIE  -D__64BIT__ -D__UNIX__ -O3 -DNDEBUG -Wno-deprecated-declarations -std=gnu++11 -MD -MT test/CMakeFiles/Test.dir/ssc_test/cmod_fuelcell_test.cpp.o -MF CMakeFiles/Test.dir/ssc_test/cmod_fuelcell_test.cpp.o.d -o CMakeFiles/Test.dir/ssc_test/cmod_fuelcell_test.cpp.o -c /home/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_fuelcell_test.cpp
In file included from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-message.h:55:0,
                 from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest-assertion-result.h:46,
                 from /home/runner/work/ssc/ssc/googletest/googletest/include/gtest/gtest.h:59,
                 from /home/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_etes_ptes_test.cpp:34:
/home/runner/work/ssc/ssc/googletest/googletest/include/gtest/internal/gtest-port.h:696:23: error: return type specification for constructor invalid
   type(type const&) = delete;                 \
                       ^
/home/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:39:7: note: in expansion of macro ‘GTEST_DISALLOW_COPY_AND_ASSIGN_’
   int GTEST_DISALLOW_COPY_AND_ASSIGN_(\
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/ssc/ssc/ssc/test/shared_test/vs_google_test_explorer_namespace.h:57:3: note: in expansion of macro ‘NAMESPACE_GTEST_TEST_’
   NAMESPACE_GTEST_TEST_(namespace_name, test_case_name, test_name,\
   ^~~~~~~~~~~~~~~~~~~~~
/home/runner/work/ssc/ssc/ssc/test/ssc_test/cmod_etes_ptes_test.cpp:43:1: note: in expansion of macro ‘NAMESPACE_TEST’
 NAMESPACE_TEST(etes_ptes_test, EtesPtesCmod, Default_NoFinancial)
 ^~~~~~~~~~~~~~

[Matthew-Boyd]

I don't think we should remove the macro as it looks to be needed for the intended functionality, which is to allow categorization of the tests in the Visual Studio Test Explorer. Part of my recent change was to add the 'int' return type on line 39 of vs_google_test_explorer_namespace.h . However, this seems to break the code on Mac and Linux.

@sjanzou Can I please take you up on your offer to test this on Mac and Linux? And maybe try different return types or selectively set the return type via something like:
#if defined(WIN32) || defined(_WIN32) || defined(WIN32) || defined(NT)

[sjanzou]

I don't think we should remove the macro as it looks to be needed for the intended functionality, which is to allow categorization of the tests in the Visual Studio Test Explorer. Part of my recent change was to add the 'int' return type on line 39 of vs_google_test_explorer_namespace.h . However, this seems to break the code on Mac and Linux.

@sjanzou Can I please take you up on your offer to test this on Mac and Linux? And maybe try different return types or selectively set the return type via something like: #if defined(WIN32) || defined(_WIN32) || defined(WIN32) || defined(NT)

@Matthew-Boyd , thanks for the details - I am running locally on CentOS7 and MacOS 13 and will update here.

[sjanzou]

I don't think we should remove the macro as it looks to be needed for the intended functionality, which is to allow categorization of the tests in the Visual Studio Test Explorer. Part of my recent change was to add the 'int' return type on line 39 of vs_google_test_explorer_namespace.h . However, this seems to break the code on Mac and Linux.

@sjanzou Can I please take you up on your offer to test this on Mac and Linux? And maybe try different return types or selectively set the return type via something like: #if defined(WIN32) || defined(_WIN32) || defined(WIN32) || defined(NT)

@Matthew-Boyd, the #if worked fine on MacOS with the "int" removed using the old googletest. However, for the new googletest, the gtest-port.h line 270 requires c++14

We will definitely not be able to support that when building on CentOS7.

So, I am not sure if there is a path forward if we need to support the Eagle HPC configuration and the latest googletest.

The latet commit on this branch updated all workflows and CMakelist.txt to C++14 for testing.

[Matthew-Boyd]

@sjanzou Thanks a lot for looking into this and finding the limitation.

It looks like the options are to keep googletest pinned at that previous version or remove this categorization. I'm fine with either, and I may be the only one who cares about this feature. @brtietz , do you want to make the decision? If we're not keeping it pinned, I'll update the categorized tests so there's no errors.

[brtietz]

@Matthew-Boyd I put this on the meeting agenda for discussion. Some things I'm thinking about:

1. Is anyone actually building the code on Eagle, or are they just using PySAM?
2. Even if they are, are they running the tests? (is there a CMake flag we could use for the test file?)
3. We can migrate away from CentOS7 when Kestrel comes online (tentatively July)

[Matthew-Boyd]

Revisit in July '23 after we upgrade to C++14