Skip to content
This repository was archived by the owner on Nov 29, 2023. It is now read-only.

Replace backslashes with slashes to fix e2e run #205

Merged

Conversation

StefanScherer
Copy link
Member

Signed-off-by: Stefan Scherer [email protected]

- What I did

Make the PWD variable on Windows a path with forward slashes (again) instead of backslashes. I saw in older workflow runs that the path used forward slashes, then started failing running the e2e tests on Windows because of it having backslashes.

I was able to reproduce locally with MinGW64 11.2.0 installed:

$ mingw32-make -f builder.Makefile e2e
mkdir -p docker-config/scan
mkdir -p docker-config/cli-plugins
cp ./bin/docker-scan_windows_amd64.exe docker-config/cli-plugins/docker-scan.exe
# TODO: gotestsum doesn't forward ldflags to go test with golang 1.15.0, so moving back to go test temporarily
SNYK_DESKTOP_VERSION=1.827.0 SNYK_USER_VERSION=1.827.0 SNYK_OLD_VERSION=1.382.1 DOCKER_CONFIG=C:\code\scan-cli-plugin/docker-config SNYK_OLD_PATH=C:\code\scan-cli-plugin/docker-config/snyk-old SNYK_USER_PATH=C:\code\scan-cli-plugin/docker-config/snyk-user SNYK_DESKTOP_PATH=C:\code\scan-cli-plugin/docker-config/snyk-desktop go test ./e2e  -ldflags="-s -w -X github.com/docker/scan-cli-plugin/internal.GitCommit=5a67dc4 -X github.com/docker/scan-cli-plugin/internal.Version=v0.1.0-231-g5a67dc4185-dirty -X github.com/docker/scan-cli-plugin/internal/provider.ImageDigest=sha256:f9291a5310e3952369eeb8cd1c2a25f0c9fc930a3ccc88e1ea20956ad86b75a4 -X github.com/docker/scan-cli-plugin/internal/provider.SnykDesktopVersion=1.827.0"
panic: open C:codescan-cli-plugin\docker-config\cli-plugins\docker-scan.exe: The system cannot find the path specified.

- How I did it

Add a subst to replace all \ with /. Surprisingly in the Makefile I didn't had to use things like double \\ backslash, just a single one is fine here.

- How to verify it

  • have MinGW 64 11.2.0 installed
  • Run mingw32-make -f builder.Makefile e2e that failed for me without the substitution. With this fix it only complains about missing E2E vars, but it came across the path

- Description for the changelog

- A picture of a cute animal (not mandatory)

@mat007
Copy link
Member

mat007 commented Aug 3, 2022

Should this fix the CI?

@StefanScherer
Copy link
Member Author

StefanScherer commented Aug 3, 2022

Yes, I hope so. CI is only triggered in the upstream repo, but not in forks, that's why we don't see a status here.
This PR should fix the errors we can see in the "Release and Weekly Build" workflow in https://github.com/docker/scan-cli-plugin/actions.
Even if I have created this PR directly in upstream repo, we couldn't run it there, only the build-pr workflow is triggered for PR's and that runs only on Linux nodes, but not on Windows.
I suggest we merge it and #204 and then see if that can build the missing linux/arm64 binary.

Copy link
Member

@mat007 mat007 left a comment

Choose a reason for hiding this comment

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

Thanks!

@StefanScherer StefanScherer merged commit 0550571 into docker:main Aug 3, 2022
@StefanScherer StefanScherer deleted the fix-windows-e2e-backslash-problem branch August 3, 2022 13:35
@StefanScherer
Copy link
Member Author

Yes, CI is green again -> https://github.com/docker/scan-cli-plugin/actions/runs/2789915771

@mat007
Copy link
Member

mat007 commented Aug 3, 2022

So we don’t have CI on the PRs? 😞
#180 was trying to change that, right?

@StefanScherer
Copy link
Member Author

Good catch, yes. Looking at the comments in #180 I think we should just skip the e2e test in PR's to avoid providing the env variables in PR builds from random forks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants