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

Fix go get xk6 github action #2153

Merged
merged 2 commits into from
Oct 4, 2021
Merged

Fix go get xk6 github action #2153

merged 2 commits into from
Oct 4, 2021

Conversation

inancgumus
Copy link
Member

  • go get is now only being used for adding dependencies to a Go module
  • this fix uses go install instead to install the xk6 binary (instead of go get)

+ go get is only used for adding dependencies to a Go module
+ this fix uses go install to install the xk6 binary (instead of go get)
@inancgumus inancgumus requested review from mstoykov and imiric October 4, 2021 07:40
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Can you please change the go get in the all.yml as well

.github/workflows/xk6.yml Outdated Show resolved Hide resolved
@inancgumus
Copy link
Member Author

inancgumus commented Oct 4, 2021

Can you please change the go get in the all.yml as well

Do we need to keep glide anymore? It's because the following now works:

  • go install github.com/mh-cbon/go-bin-deb@master
  • go install github.com/mh-cbon/go-bin-rpm@master

Instead of:

go get -d github.com/mh-cbon/go-bin-deb \
              && (cd "$gopath/src/github.com/mh-cbon/go-bin-deb" \
              && glide install \
              && go install)

go get -d github.com/mh-cbon/go-bin-rpm \
              && (cd "$gopath/src/github.com/mh-cbon/go-bin-rpm" \
              && glide install \
              && go install)

Should we keep go getting goversioninfo as it's doing some other stuff (below). Then again, I think the correct approach is to install it with go install, but I can't test it on my Mac.

(cd && GO111MODULE=on go get github.com/josephspurrier/goversioninfo/cmd/[email protected])
IFS=. read -a version <<< "$(echo $VERSION | sed 's:[^0-9\.]::g')"
# Need a blank versioninfo.json for the CLI overrides to work.
echo '{}' > versioninfo.json
goversioninfo -64 \
  -platform-specific=true \
  -charset="1200" \
  -company="Load Impact AB" \
  -copyright="© Load Impact AB. Licensed under AGPL." \
  -description="A modern load testing tool, using Go and JavaScript" \
  -icon=packaging/k6.ico \
  -internal-name="k6" \
  -original-name="k6.exe" \
  -product-name="k6" \
  -translation="0x0409" \
  -ver-major="${version[0]}" \
  -ver-minor="${version[1]}" \
  -ver-patch="${version[2]}" \
  -product-version="${VERSION#v}"

I tried this with go install but I couldn't get the version info (probably because I'm on a Mac):

go install github.com/josephspurrier/goversioninfo/cmd/[email protected]
# this doesn't work on my mac so I can't test it
# IFS=. read -a version <<< "$(echo $VERSION | sed 's:[^0-9\.]::g')"
echo '{}' > versioninfo.json
goversioninfo -64 \
    ....

@mstoykov @imiric

So we can test even with unreleased versions of xk6.
@yorugac
Copy link
Contributor

yorugac commented Oct 4, 2021

Just a note, I think this PR should fix the current failure in Github actions for master branch at test-xk6 (tip, macos-latest) job.

@inancgumus
Copy link
Member Author

test-xk6 (tip, macos-latest)

I couldn't find that branch?

@yorugac
Copy link
Contributor

yorugac commented Oct 4, 2021

test-xk6 is a job name, it just failed on master branch here: https://github.com/grafana/k6/runs/3788611719

Also, a suggestion: how about adding fail-fast: false to the test-xk6 as well? As is done for test-prev and test-current-cov in all.yml

Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Given that this currently breaks CI and the others will not be relevant until we move to 1.18 to release I am for merging this as is and @inancgumus please open at least a very basic issue to actually change the rest so we don't forget it

@inancgumus inancgumus merged commit e0d8d93 into master Oct 4, 2021
@inancgumus inancgumus deleted the fix/xk6-go-install branch October 4, 2021 11:24
@mstoykov mstoykov added this to the v0.35.0 milestone Oct 26, 2021
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.

4 participants