feat: add semantic version utilities for e2e tests #2502
Conversation
colin-axner
requested review from
AdityaSripal,
crodriguezvega,
seantking,
charleenfei,
damiannolan and
chatton
as code owners
e2e/testsuite/semver_test.go
Outdated
| {"supported semantic version", "v6.1.0", true}, | ||
| {"supported semantic version", "v7.1.0", true}, | ||
| {"supported semantic version", "v22.5.1", true}, | ||
| {"supported semantic version with v", "2.5.0", true}, |
There was a problem hiding this comment.
| {"supported semantic version with v", "2.5.0", true}, | |
| {"supported semantic version without v", "2.5.0", true}, |
e2e/testsuite/semver.go
Outdated
| } | ||
| } | ||
|
|
||
| func ParseChainVersion(version string) (Version, bool) { |
There was a problem hiding this comment.
Is this function is only used in IsSupported, can it be unexported? If it cannot, can we add a godoc then?
| // or is greater than or equal to the list of minor releases it was included in. | ||
| func (fr FeatureReleases) IsSupported(versionStr string) bool { | ||
| // assume any non semantic version formatted version support the feature | ||
| // this will be useful during development of the e2e test with the new feature |
There was a problem hiding this comment.
So this is to support images with names of working branches, right?
There was a problem hiding this comment.
right, this will mean a version of pr-1234 will "support" a feature.
There was a problem hiding this comment.
All right... maybe this is an edge case, but if we have a PR for a branch that does not support the feature (for example, a backport to release/v2.4.x) then the test will fail, right? Because this will wrongly assume that the PR supports the feature... Although maybe this is not a problem if the metadata tests are only run as part of the compatibility check.
e2e/testsuite/semver.go
Outdated
| return Version{ | ||
| Major: major, | ||
| Minor: minor, | ||
| }, true |
There was a problem hiding this comment.
The majority of the parsing logic here seems to be duplicating semver.Major and semver.Minor
There was a problem hiding this comment.
ah! I was looking for that, wasn't sure it existed, looks to be importable under "semver"?
There was a problem hiding this comment.
it looks like the import path is "golang.org/x/mod/semver"
|
After the refactor, the custom types to represent versions were removed in favour of strings which are natively supported by the |
There was a problem hiding this comment.
ACK, thanks @chatton! Feel free to merge for me whenever you feel ready
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes