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

scripts: Add tests for release scripts #13981

Merged
merged 2 commits into from
Apr 26, 2022
Merged

Conversation

serathius
Copy link
Member

Ref #13980

@serathius serathius force-pushed the test-release branch 5 times, most recently from 31caa01 to 6a97d22 Compare April 24, 2022 14:00
@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2022

Codecov Report

Merging #13981 (dd98f1c) into main (a379ffc) will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13981      +/-   ##
==========================================
- Coverage   72.67%   72.52%   -0.16%     
==========================================
  Files         469      470       +1     
  Lines       38413    38497      +84     
==========================================
+ Hits        27918    27920       +2     
- Misses       8722     8789      +67     
- Partials     1773     1788      +15     
Flag Coverage Δ
all 72.52% <ø> (-0.16%) ⬇️

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

Impacted Files Coverage Δ
server/proxy/grpcproxy/register.go 69.76% <0.00%> (-9.31%) ⬇️
client/pkg/v3/testutil/leak.go 62.83% <0.00%> (-7.08%) ⬇️
client/v3/leasing/util.go 91.66% <0.00%> (-6.67%) ⬇️
raft/rafttest/node.go 95.00% <0.00%> (-5.00%) ⬇️
server/storage/mvcc/watchable_store.go 85.86% <0.00%> (-3.99%) ⬇️
server/etcdserver/api/v3rpc/util.go 70.96% <0.00%> (-3.23%) ⬇️
server/etcdserver/api/v3rpc/member.go 93.54% <0.00%> (-3.23%) ⬇️
server/etcdserver/api/v3rpc/interceptor.go 74.47% <0.00%> (-3.13%) ⬇️
client/v3/leasing/cache.go 88.88% <0.00%> (-2.78%) ⬇️
client/v3/experimental/recipes/key.go 75.34% <0.00%> (-2.74%) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a379ffc...dd98f1c. Read the comment docs.

@serathius serathius force-pushed the test-release branch 3 times, most recently from 7aa9609 to 3746af9 Compare April 24, 2022 14:08
@serathius
Copy link
Member Author

Removing other workflows to speed up iteration

@serathius serathius force-pushed the test-release branch 18 times, most recently from 105549c to d26d345 Compare April 24, 2022 20:24
@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2022

I see lots of workflow yaml files are removed, did you intentionally remove them?

tools/mod/go.mod Outdated Show resolved Hide resolved
@serathius
Copy link
Member Author

Removal of other workflows was just to speed up iteration.

@serathius serathius force-pushed the test-release branch 2 times, most recently from bee22cb to 61a7c09 Compare April 25, 2022 07:46
@serathius
Copy link
Member Author

cc @ptabor @spzala @ahrtr
Ready for review

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

@serathius lgtm, you know the release the best :) Thanks!

Name-Email: github-action@etcd.io
Expire-Date: 0
EOF
BRANCH=main ./scripts/release.sh --no-upload --no-docker-push 3.6.99
Copy link
Member

Choose a reason for hiding this comment

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

Just curious about the value, 3.6.99? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume it's just value bigger than any 3.6 patch version released so far.

Copy link
Member Author

Choose a reason for hiding this comment

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

Needed to pick a version to release script to run. I used 3.6 as I plan to add tests for other branches in next PRs. Picked .99 as wanted the version to correct but easy to distinguish from normal release. We could pick any other version that would fulfills those requirements.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Thanks @serathius

@spzala
Copy link
Member

spzala commented Apr 25, 2022

I see lots of workflow yaml files are removed, did you intentionally remove them?

Yeah, I was curious too thinking it's impact on CI and was thinking to ask Marek about it to understand it which he already answered now :)

@@ -102,7 +102,7 @@ main() {

# Check go version.
local go_version current_go_version
go_version="go$(run_go_tool "github.com/mikefarah/yq/v3" read .github/workflows/build.yaml "jobs.build.steps[1].with.go-version")"
go_version="go$(grep go-version .github/workflows/build.yaml | awk '{print $2}' | tr -d '"')"
Copy link
Contributor

Choose a reason for hiding this comment

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

curious. Why grep ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was not able to manage to get the go tool to run in CI. It always failed saying that dependency is missing. It would be even harder if I wanted to use this approach to support old release branches.

Name-Email: github-action@etcd.io
Expire-Date: 0
EOF
BRANCH=main ./scripts/release.sh --no-upload --no-docker-push 3.6.99
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume it's just value bigger than any 3.6 patch version released so far.

Name-Email: github-action@etcd.io
Expire-Date: 0
EOF
BRANCH=main ./scripts/release.sh --no-upload --no-docker-push 3.6.99
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add DRY_RUN=true here. I know it's the default... but I think it would improve the readability.

Copy link
Member Author

Choose a reason for hiding this comment

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

sg

@serathius serathius merged commit 82a258d into etcd-io:main Apr 26, 2022
@serathius serathius deleted the test-release branch June 15, 2023 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants