-
Notifications
You must be signed in to change notification settings - Fork 39
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
Improve istioversion and e2e test #686
base: main
Are you sure you want to change the base?
Conversation
/test lint |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #686 +/- ##
==========================================
+ Coverage 74.31% 74.40% +0.08%
==========================================
Files 43 43
Lines 2597 2637 +40
==========================================
+ Hits 1930 1962 +32
- Misses 574 579 +5
- Partials 93 96 +3 ☔ View full report in Codecov by Sentry. |
|
||
// getBaseAndNewVersion returns the base and new Istio versions to be used in the tests. | ||
// It will take the latest Major.Minor version and get the two newest patch of that Major.Minor version. | ||
func getBaseAndNewVersion() (string, string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this new logic? Can't we just use .Old and .New like before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and if we replace .Old and .New, we should remove them and put this func (or a similar one) in the istioversion package 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because .Old and.New can be different minor versions. Do we want to test also upgrades between versions? I mean, things can be different between minor versions
/test lint |
|
||
// getBaseAndNewVersion returns the base and new Istio versions to be used in the tests. | ||
// It will take the latest Major.Minor version and get the two newest patch of that Major.Minor version. | ||
func getBaseAndNewVersion() (string, string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be in version.go?
continue | ||
} | ||
|
||
newVer, err1 := semver.NewVersion(group[0].Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Name
is the version name, not the semantic version number. You should probably use .Version
here.
0704bb5
to
f32e191
Compare
5766bd2
to
897be69
Compare
Signed-off-by: Francisco H <frherrer@redhat.com>
Co-authored-by: Marko Lukša <mluksa@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com>
897be69
to
12e7190
Compare
Signed-off-by: Francisco H <frherrer@redhat.com>
Signed-off-by: Francisco H <frherrer@redhat.com>
Signed-off-by: Francisco H <frherrer@redhat.com>
Fixes #645. This issue was related to not running against all the versions in the versions.yaml file |
What type of PR is this?
What this PR does / why we need it:
This PR adds a new function to get all the patch versions from each Major.Minor version in the versions.yaml to be used later by the e2e test to iterate over only the latest patch version on each Minor.
This will decrease the execution time of the e2e framework because before we where executing a lot of test for all the versions inside the versions.yaml file, now, we are only going to run the scenario for the latest patch for each Minor version
Which issue(s) this PR fixes:
Fixes #681
Related Issue/PR #
Additional information: