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

release(upgradetest): confirm postrelease version is newer than the latest sourcegraph version #62463

Merged
merged 2 commits into from May 14, 2024

Conversation

BolajiOlajide
Copy link
Contributor

Last week, I ran into an obscure error where an internal release build had it's release tests failing.

Context

This led to an hypothesis where we believe the version I was trying to release 5.3.666 was assumed by the tests to be behind the latest version 5.3.12303. I added this check to ensure this doesn't repeat itself.

Test plan

Will test during the release testing later.

@BolajiOlajide BolajiOlajide requested review from DaedalusG and a team May 6, 2024 16:33
@BolajiOlajide BolajiOlajide self-assigned this May 6, 2024
@cla-bot cla-bot bot added the cla-signed label May 6, 2024
if latestFullVer == nil || tag.GreaterThan(latestFullVer) {
latestFullVer = tag
if latestFullVer == nil || v.GreaterThan(latestFullVer) {
latestFullVer = v
}
latestMinorVer, err = semver.NewVersion(fmt.Sprintf("%d.%d.0", latestFullVer.Major(), latestFullVer.Minor()))
if err != nil {
return nil, nil, nil, nil, nil, nil, err
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DaedalusG I took the liberty of tweaking the logic here as I don't think we need the second loop here. Moving this logic after appending to validTags will achieve the same result albeit faster.

Let me know if you have concerns

}
latestMinorVer, err = semver.NewVersion(fmt.Sprintf("%d.%d.0", latestFullVer.Major(), latestFullVer.Minor()))
if err != nil {
return nil, nil, nil, nil, nil, nil, err
}
}

semverPostRelease := semver.MustParse(postRelease)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably wrap this in a check to make sure the postRelease flag is set. There are ways to run this test without specifying postRelease targets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see. Added an if check to only do this is the postRelease flag is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DaedalusG Can you re-review when you have some time?

@BolajiOlajide BolajiOlajide added the backport 5.4.0 label used to backport PRs to the 5.4.0 release branch label May 14, 2024
@BolajiOlajide BolajiOlajide merged commit c1e29f0 into main May 14, 2024
12 checks passed
@BolajiOlajide BolajiOlajide deleted the bo/check-postrelease-version-constraint branch May 14, 2024 08:15
sourcegraph-release-guild-bot pushed a commit that referenced this pull request May 14, 2024
…atest sourcegraph version (#62463)

* check for post release version

* check if postRelease is provided

(cherry picked from commit c1e29f0)
BolajiOlajide added a commit that referenced this pull request May 14, 2024
… newer than the latest sourcegraph version (#62651)

release(upgradetest): confirm postrelease version is newer than the latest sourcegraph version (#62463)

* check for post release version

* check if postRelease is provided

(cherry picked from commit c1e29f0)

Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 5.4.0 label used to backport PRs to the 5.4.0 release branch cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants