GEODE-9549: enable netcore tests in CI #867
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Adds post-build steps to netcore directory to move output files from source directory to build directory - Enable test running job in yml
- Alpine image blows up trying to read SSH key file, because reasons.
- Still no idea what's gonna happen tho
- last tag we can try is debian_component_based
- This simplifies the cmake config, and makes the post-build file copy steps more reliable
- Two potential issues: first, not capitalizing subprojects means we potentially have two cmake targets called 'netcore'. This is probably bad. Second, seeing sharing violations and weirdness on various platforms, it looks like NuGet restore being run on two projects at once. Try to fix this by making NetCore.Test depend on NetCore.
- We're copying the output files to the binary dir prior to install, so that's where install should pick up the library
- Turns out 'dotnet test' won't run in the output directory, it literally requires projects to be built in place.
- Also added a couple of missing CMakeLists.txt files.
- Shouldn't be installing in the test dirs (copy/paste error) - Give Java project a reasonable name
- Windows scripts and tests work properly now
- Should work on all platforms now
- parallel builds on Linux would hit sharing violations, apparently because projects were being built more than once and simultaneously.
- Eliminates need for start/stop scripts, all done via ctest now
- no longer in use or necessary
- Still getting sharing violations on parallel builds, so need to ensure none of these build at the same time.
- file copy for C bindings library had wrong destination dir
- Forgot this one, and it's failing RAT check
- binary path doesn't actually include 'x64', not sure how it got that way on my dev machine
This reverts commit f8c0392.
- Still didn't get it right, they sure use a lot of subdirectories
- Tests actually run this way, which is nice
- command will default to Debug config unless specified, which causes problems on release builds
- dotnet command line switches are case-sensitive, it turns out
- Removed for debugging .net core test running
pdxcodemonkey
requested review from
jake-at-work,
mreddington,
mmartell and
echobravopapa
echobravopapa
approved these changes
Sep 20, 2021
jake-at-work
requested changes
Sep 20, 2021
ci/lib/templates.lib.yml
Outdated
| @@ -222,7 +221,8 @@ plan: | |||
| - #@ remote_task("net-integration-tests", config.config, ctest_bash_task("build/clicache/integration-test2"), "1h", build.params) | |||
| - #@ remote_task("net-legacy-integration-tests", config.config, ctest_bash_task("build/clicache/integration-test", timeout=500, parallel=4), "2h", build.params) | |||
| #@ end | |||
| #! - #@ remote_task("netcore-tests", config.config, dotnet_bash_task("source/netcore/NetCore.Test"), "10m", build.params) | |||
| - #@ remote_task("netcore-integration-test", config.config, ctest_bash_task("build/netcore/NetCore.Test", config.config, parallel=1), "10m", build.params) | |||
There was a problem hiding this comment.
YES!!! Though I would love to see the directory/project name match the integration test designation.
netcore/CMakeLists.txt
Outdated
| if(DOTNET AND INCLUDE_DOTNET_CORE) | ||
| add_custom_target(netcore ALL COMMAND ${DOTNET} build --configuration $<CONFIG> WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/NetCore DEPENDS apache-geode-c VERBATIM) | ||
| add_custom_target(netcore-test ALL COMMAND ${DOTNET} build --configuration $<CONFIG> WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/NetCore.Test DEPENDS netcore VERBATIM) | ||
| add_subdirectory(NetCore) |
There was a problem hiding this comment.
You should probably .NET Core check around the add_subdirectory calls. This prevents them from even being processed on when not enabled.
if(DOTNET AND INCLUDE_DOTNET_CORE)
endif()
| # limitations under the License. | ||
| project(netcore-session-integrationtest LANGUAGES NONE) | ||
|
|
||
| if(DOTNET AND INCLUDE_DOTNET_CORE) |
There was a problem hiding this comment.
My moving this out to the parent it avoids the issue of tests being added in this file for targets never built.
- Causes problems between Linux and Windows
jake-at-work
approved these changes
Sep 21, 2021
- build appears to be good, tests run
davebarnes97
pushed a commit
to davebarnes97/geode-native
that referenced
this pull request
Feb 9, 2022
* GEODE-9549: Enable .net core tests in CI - Enable test running job in yml * GEODE-9549: Build NetCore and NetCore.Test projects individually - This simplifies the cmake config, and makes the post-build file copy steps more reliable * GEODE-9549: Copy C bindings lib to binary dir for tests - Give Java project a reasonable name * GEODE-9549: Use ctest to run netcore tests - Eliminates need for start/stop scripts, all done via ctest now * GEODE-9549: Make .net core project build in sequence - Still getting sharing violations on parallel builds, so need to ensure none of these build at the same time. Co-authored-by: Jacob Barrett <[email protected]>
davebarnes97
pushed a commit
to davebarnes97/geode-native
that referenced
this pull request
Feb 9, 2022
* GEODE-9549: Enable .net core tests in CI - Enable test running job in yml * GEODE-9549: Build NetCore and NetCore.Test projects individually - This simplifies the cmake config, and makes the post-build file copy steps more reliable * GEODE-9549: Copy C bindings lib to binary dir for tests - Give Java project a reasonable name * GEODE-9549: Use ctest to run netcore tests - Eliminates need for start/stop scripts, all done via ctest now * GEODE-9549: Make .net core project build in sequence - Still getting sharing violations on parallel builds, so need to ensure none of these build at the same time. Co-authored-by: Jacob Barrett <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.