Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-discover unit tests #10922

Merged
merged 5 commits into from
Feb 5, 2025
Merged

Auto-discover unit tests #10922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d19f088
Stop manually parsing for gtest, use discovery instead
Myoldmopar Feb 4, 2025
590cfde
Either fix or comment out disabled unit tests
Myoldmopar Feb 4, 2025
1689f26
Comment failing test
Myoldmopar Feb 5, 2025
34de646
Avoiding 2 tests on GHA, eliminating a test binary, and fixing a seg …
Myoldmopar Feb 5, 2025
12b75e3
Merge remote-tracking branch 'origin/develop' into TryDiscoverTests
Myoldmopar Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions cmake/ProjectMacros.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,5 @@
include(CMakeParseArguments)

# Add google tests macro
macro(ADD_GOOGLE_TESTS executable)
foreach(source ${ARGN})
string(REGEX MATCH .*cpp|.*cc source "${source}")
if(source)
file(READ "${source}" contents)
string(REGEX MATCHALL "TEST_?F?\\(([A-Za-z_0-9 ,]+)\\)" found_tests ${contents})
foreach(hit ${found_tests})
string(REGEX REPLACE ".*\\(( )*([A-Za-z_0-9]+)( )*,( )*([A-Za-z_0-9]+)( )*\\).*" "\\2.\\5" test_name ${hit})
add_test(NAME ${test_name} COMMAND "${executable}" "--gtest_filter=${test_name}")
endforeach(hit)
endif()
endforeach()
endmacro()
include(GoogleTest)

# Create source groups automatically based on file path
macro(CREATE_SRC_GROUPS SRC)
Expand Down Expand Up @@ -62,7 +48,11 @@ macro(CREATE_TEST_TARGETS BASE_NAME SRC DEPENDENCIES USE_PCH)

target_link_libraries(${BASE_NAME}_tests PRIVATE ${ALL_DEPENDENCIES} gtest)

add_google_tests(${BASE_NAME}_tests ${SRC})
gtest_discover_tests(${BASE_NAME}_tests
DISCOVERY_TIMEOUT 30
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced that block of "manual" source code parsing with just a call to let gtest discover the tests and add them to ctest. Excellence.

endif()
endmacro()

Expand Down
17 changes: 10 additions & 7 deletions src/EnergyPlus/WindowManagerExteriorOptical.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,16 @@ namespace Window {
constrSh.effShadeBlindEmi[iSlatAng] = EpsShIR * (1.0 + RhoGlIR * TauShIR / (1.0 - RhoGlIR * RhoShIR));
constrSh.effGlassEmi[iSlatAng] = EpsGlIR * TauShIR / (1.0 - RhoGlIR * RhoShIR);
}

surfShade.effShadeEmi = Interp(constrSh.effShadeBlindEmi[surfShade.blind.slatAngIdxLo],
constrSh.effShadeBlindEmi[surfShade.blind.slatAngIdxHi],
surfShade.blind.slatAngInterpFac);
surfShade.effGlassEmi = Interp(constrSh.effGlassEmi[surfShade.blind.slatAngIdxLo],
constrSh.effGlassEmi[surfShade.blind.slatAngIdxHi],
surfShade.blind.slatAngInterpFac);
// The following surface assignments previously lived in the WindowManagerExteriorThermal file.
// They were moved here during the Material refactor, however I found that the array index values Lo/Hi
// were making it here still as -1, and this was causing a difficult to diagnose segfault.
// I'm moving these back over there, but open to suggestion if they need to be here.
// surfShade.effShadeEmi = Interp(constrSh.effShadeBlindEmi[surfShade.blind.slatAngIdxLo],
// constrSh.effShadeBlindEmi[surfShade.blind.slatAngIdxHi],
// surfShade.blind.slatAngInterpFac);
// surfShade.effGlassEmi = Interp(constrSh.effGlassEmi[surfShade.blind.slatAngIdxLo],
// constrSh.effGlassEmi[surfShade.blind.slatAngIdxHi],
// surfShade.blind.slatAngInterpFac);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment explains it, but basically, on my Apple Silicon, in a release build, this was throwing a segfault because the array indices were still -1 here. I just moved them to where they were assigned before the Material refactor, and it's all happy locally. We'll see how CI likes it.

} // End of check if interior shade or interior blind
} // End of surface loop
} // InitWCE_SimplifiedOpticalData()
Expand Down
9 changes: 7 additions & 2 deletions src/EnergyPlus/WindowManagerExteriorThermal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,15 @@ namespace Window {
}
SurfInsideTemp = aTemp - Constant::Kelvin;
if (ANY_INTERIOR_SHADE_BLIND(state.dataSurface->SurfWinShadingFlag(SurfNum))) {
auto const &surfShade = state.dataSurface->surfShades(SurfNum);
auto &surfShade = state.dataSurface->surfShades(SurfNum);
Real64 EffShBlEmiss = surfShade.effShadeEmi;
Real64 EffGlEmiss = surfShade.effGlassEmi;

surfShade.effShadeEmi = Interp(construction.effShadeBlindEmi[surfShade.blind.slatAngIdxLo],
construction.effShadeBlindEmi[surfShade.blind.slatAngIdxHi],
surfShade.blind.slatAngInterpFac);
surfShade.effGlassEmi = Interp(construction.effGlassEmi[surfShade.blind.slatAngIdxLo],
construction.effGlassEmi[surfShade.blind.slatAngIdxHi],
surfShade.blind.slatAngInterpFac);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved these back here, where they were before the Material refactor. Let's see.

state.dataSurface->SurfWinEffInsSurfTemp(SurfNum) =
(EffShBlEmiss * SurfInsideTemp + EffGlEmiss * (state.dataWindowManager->thetas[2 * totSolidLayers - 3] - Constant::Kelvin)) /
(EffShBlEmiss + EffGlEmiss);
Expand Down
Loading