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.

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

Create windows-cmake.yml #327

Merged
merged 3 commits into from
Jul 10, 2023
Merged

Create windows-cmake.yml #327

merged 3 commits into from
Jul 10, 2023

Conversation

ZigRazor
Copy link
Owner

@ZigRazor ZigRazor commented Jul 6, 2023

Added Windows CMake CI/CD

@github-actions github-actions bot added the automated workflows Automated Workflows label Jul 6, 2023
@ghost
Copy link

ghost commented Jul 6, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #327 (c706385) into master (4d60bf6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #327   +/-   ##
=======================================
  Coverage   97.24%   97.24%           
=======================================
  Files          54       54           
  Lines        7907     7907           
=======================================
  Hits         7689     7689           
  Misses        218      218           
Flag Coverage Δ
unittests 97.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@ZigRazor
Copy link
Owner Author

ZigRazor commented Jul 6, 2023

@nrkramer can you correct the cmake to let the MSVC compile? The problem seems to be the linking of pthread library, that obviously is not present in windows

@github-actions github-actions bot added the test Something about test label Jul 6, 2023
for compiling on windows
@ZigRazor
Copy link
Owner Author

ZigRazor commented Jul 6, 2023

OK @nrkramer and @sbaldu, I correct the compilation on Windows, now some tests failed, can you see why?

Comment on lines +65 to +67
if(NOT gtest_disable_pthreads)
target_link_libraries(test_exe PUBLIC pthread)
endif(NOT gtest_disable_pthreads)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect, thanks!

@nrkramer
Copy link
Collaborator

nrkramer commented Jul 8, 2023

OK @nrkramer and @sbaldu, I correct the compilation on Windows, now some tests failed, can you see why?

@ZigRazor

The following tests FAILED:
	 13 - test_partition (Failed)
	 18 - test_DOT (Failed)

These two tests rely on CXXGraph::EdgeSet_T, which has the weirdness we see in #326.
These tests also fail for me at-desk.

I think I'll need to experiment a bit when I have the time to hone in on where the problem lies. For example, creating a std::unordered_set with a similar hash. Since we know the hash is different, perhaps the problem lies within Microsft's STL implementation and we'll need to open an issue there (https://github.com/microsoft/STL). There's tests I can do to confirm that.

Otherwise, if anyone has any other ideas, they can push commits to this PR to see if the tests begin passing.

@ZigRazor
Copy link
Owner Author

yes, I think there is a bug in the STL, or in any case, we are in case that is not covered by the Microsoft STL.

@ZigRazor
Copy link
Owner Author

I merge this PR, in this way we can check in the future the problem, in any case for the moment the "Build and Tests" in windows CI/CD will fail.

@ZigRazor ZigRazor merged commit fa2c3e9 into master Jul 10, 2023
18 of 19 checks passed
@ZigRazor ZigRazor deleted the Windows-Cmake-Tests branch January 9, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automated workflows Automated Workflows test Something about test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants