Fix windows tests #55
Merged
Conversation
This file contains hidden or 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
ebroberson
reviewed
Jan 27, 2023
| var builderArgs []string | ||
| var discoveredTarPath string | ||
| if runtime.GOOS == "windows" { | ||
| builderArgs = []string{"-tags=windows2012R2"} |
There was a problem hiding this comment.
Is there a reason we're using 2012R2 instead of something more recent?
There was a problem hiding this comment.
This comes from the file we deleted as part of this change:
MarcPaquette
approved these changes
Jan 27, 2023
These tests were written when Windows did not include a tar.exe, so it had to be downloaded for Windows tests. Newer versions of Windows include tar.exe, so we've updated the tests to fall back to trying a local tar.exe Signed-off-by: David Sabeti <[email protected]>
These tests were failing on Windows when the test executable was explicitly installed as 'hello', as opposed to using the native go build output of 'hello.exe'. Changing all invocations to use the 'basename' of the go built executable (in the case of windows an '.exe' extension is added) allowed the failing tests to succeed. Signed-off-by: David Sabeti <[email protected]>
Signed-off-by: Aram Price <[email protected]>
dsabeti
force-pushed
the
fix-windows-tests
branch
from
3877a3c to
0d85ddc
Compare
MarcPaquette
pushed a commit
to MarcPaquette/community
that referenced
this pull request
May 25, 2023
…prover Diego Approver cloudfoundry/bbs#64 cloudfoundry/bbs#68 cloudfoundry/diego-dockerfiles@3aa03b8 cloudfoundry/diego-dockerfiles@77dccfa cloudfoundry/diego-dockerfiles@c69911c cloudfoundry/diego-dockerfiles@df0aed2 cloudfoundry/diego-release@05dcaba cloudfoundry/diego-release@390d5d7 cloudfoundry/diego-release@4d5e6a1 cloudfoundry/diego-release@5158007 cloudfoundry/diego-release#740 cloudfoundry/docs-cloudfoundry-concepts#181 cloudfoundry/dontpanic#39 cloudfoundry/executor#76 cloudfoundry/executor#80 cloudfoundry/grace#3 cloudfoundry/healthchecker-release#2 cloudfoundry/wg-app-platform-runtime-ci@03fa33d cloudfoundry/wg-app-platform-runtime-ci@3affeda PR Reviews: cloudfoundry/buildpackapplifecycle#55 cloudfoundry/cacheddownloader#18 cloudfoundry/diego-release#689 cloudfoundry/diego-release#690 cloudfoundry/diego-release#704 cloudfoundry/diego-release#728
ameowlia
pushed a commit
to cloudfoundry/community
that referenced
this pull request
May 30, 2023
…prover Diego Approver cloudfoundry/bbs#64 cloudfoundry/bbs#68 cloudfoundry/diego-dockerfiles@3aa03b8 cloudfoundry/diego-dockerfiles@77dccfa cloudfoundry/diego-dockerfiles@c69911c cloudfoundry/diego-dockerfiles@df0aed2 cloudfoundry/diego-release@05dcaba cloudfoundry/diego-release@390d5d7 cloudfoundry/diego-release@4d5e6a1 cloudfoundry/diego-release@5158007 cloudfoundry/diego-release#740 cloudfoundry/docs-cloudfoundry-concepts#181 cloudfoundry/dontpanic#39 cloudfoundry/executor#76 cloudfoundry/executor#80 cloudfoundry/grace#3 cloudfoundry/healthchecker-release#2 cloudfoundry/wg-app-platform-runtime-ci@03fa33d cloudfoundry/wg-app-platform-runtime-ci@3affeda PR Reviews: cloudfoundry/buildpackapplifecycle#55 cloudfoundry/cacheddownloader#18 cloudfoundry/diego-release#689 cloudfoundry/diego-release#690 cloudfoundry/diego-release#704 cloudfoundry/diego-release#728
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Windows tests were failing for a few reasons:
TAR_URLneeded to be specified, but the historical location for a Windowstar.exewas deleted. The good news is that we can use localtar.exeavailable on most Windows VMs (example, the Diego team's CI's concourse worker)TAR_URLis specified, the tests will still download and use the executable there. This could be useful if it becomes important to test with a specific version oftar.exe.hellovshello.exe). Our changes preserve the native executable as determined bygo build. This got the tests to pass.Bonus content:
@aramprice