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

ci: add windows to matrix tests #139

Merged
merged 1 commit into from
Nov 11, 2023
Merged

Conversation

glimchb
Copy link
Contributor

@glimchb glimchb commented Oct 31, 2023

Resolved #140

Signed-off-by: Boris Glimcher Boris.Glimcher@emc.com

@glimchb glimchb changed the title ci: add win and mac to matrix tests ci: add windows to matrix tests Oct 31, 2023
@glimchb glimchb marked this pull request as draft October 31, 2023 16:30
@glimchb glimchb force-pushed the windows branch 3 times, most recently from a43b239 to 1491c36 Compare October 31, 2023 18:21
@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2023

Codecov Report

Merging #139 (9d2132b) into master (087cefb) will not change coverage.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff           @@
##           master     #139   +/-   ##
=======================================
  Coverage   65.26%   65.26%           
=======================================
  Files          25       25           
  Lines        2096     2096           
=======================================
  Hits         1368     1368           
  Misses        601      601           
  Partials      127      127           

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@glimchb glimchb force-pushed the windows branch 4 times, most recently from 9c808dd to 7ceedb7 Compare October 31, 2023 20:04
@glimchb glimchb marked this pull request as ready for review October 31, 2023 20:08
@glimchb
Copy link
Contributor Author

glimchb commented Oct 31, 2023

fails on

Testing cockroachdb
Unable to find image 'cockroachdb/cockroach:latest' locally
latest: Pulling from cockroachdb/cockroach
docker: no matching manifest for windows/amd64 10.0.20348 in the manifest list entries.
See 'docker run --help'.

Error: exit status 125
Error: Process completed with exit code 1.

so trying to skip docker images for now on windows...

after this is merged, we can start re-enable windows images as they appear, for example https://hub.docker.com/_/mongo/tags?page=1&name=windows

Fixes philippgille#140

Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb glimchb marked this pull request as ready for review November 7, 2023 02:25
Copy link
Owner

@philippgille philippgille left a comment

Choose a reason for hiding this comment

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

Awesome, thank you so much! 👍

@@ -89,6 +90,11 @@ func testImpl(impl string) error {
return errors.New("unknown `gokv.Store` implementation")
}

// TODO: until docker images for windows appear, skip those test for windows
Copy link
Owner

@philippgille philippgille Nov 11, 2023

Choose a reason for hiding this comment

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

It's unlikely that there will be Windows specific images for many of the services, so I was hoping we could either

  • Switch the Docker daemon to Linux mode (like this), but apparently the GitHub Action Windows runners don't support it (source)
  • Or run Docker in WSL 2, but apparently the GitHub Action Windows runners only support WSL 1 (source)

So we're out of luck. Let's see if the situation improves in the future. For now it's already a win to test the other gokv stores that don't rely on services running in Docker 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found one for mongo
https://hub.docker.com/_/mongo/tags?page=1&name=windows
but yeah, was exploring both options for above points you made as well

@philippgille philippgille merged commit 365cd6e into philippgille:master Nov 11, 2023
10 checks passed
@glimchb glimchb deleted the windows branch November 13, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: add windows and macos
3 participants