-
Notifications
You must be signed in to change notification settings - Fork 200
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
Adding unit test for Version method #561
Conversation
Codecov Report
@@ Coverage Diff @@
## master #561 +/- ##
=========================================
+ Coverage 31.58% 31.6% +0.01%
=========================================
Files 138 138
Lines 10943 10943
=========================================
+ Hits 3456 3458 +2
+ Misses 7225 7223 -2
Partials 262 262
Continue to review full report at Codecov.
|
pkg/install/v1alpha1/version_test.go
Outdated
validatedVersion := Version(string(tt.version)) | ||
if !reflect.DeepEqual(string(validatedVersion), tt.expversion) { | ||
t.Errorf("Version error %v", validatedVersion) | ||
} |
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.
@Pensu thanks for the PR ..... can we make error more clearer. for example:
t.Errorf ( "version error, got version: %v , expected version: %v ", validatedVersion, tt.expversion)
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.
@prateekpandey14 Made the changes. PTAL.
pkg/install/v1alpha1/version_test.go
Outdated
func TestVersion(t *testing.T) { | ||
tests := map[string]struct { | ||
version string | ||
expversion 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.
expVersion
or expectedVersion
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.
@princerachit Just wanted to keep it short, so abbreviated expected to exp, you want to put expected?
pkg/install/v1alpha1/version_test.go
Outdated
|
||
func TestVersion(t *testing.T) { | ||
tests := map[string]struct { | ||
version 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.
this is also good but maybe inputVersion
is better. what do you think @prateekpandey14 ?
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.
Please address the changes.
pkg/install/v1alpha1/version_test.go
Outdated
for name, tt := range tests { | ||
t.Run(name, func(t *testing.T) { | ||
validatedVersion := Version(string(tt.version)) | ||
if !reflect.DeepEqual(string(validatedVersion), tt.expversion) { |
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.
Please avoid reflect to do a string comparison.
Signed-off-by: Peeyush Gupta <[email protected]>
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.
Lgtm
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.
LGTM
Refer: [GOVERNANCE.md](https://github.com/openebs/openebs/blob/master/GOVERNANCE.md). The following community members have been helping with enhancing and testing of several areas of the OpenEBS project that broadly fall in the purview of the control plane. Based on their past contributions and their interest/commitment to contributing to OpenEBS, we take pride in adding them as reviewers to the OpenEBS project. The list of community members in alphabetical order are: - Mehran Kholdi for the contributions to the Local PV and control plane in general. ``` - openebs#2975 - 40+ commits in https://github.com/openebs/rawfile-localpv ``` - Michael Fornaro for the contributions to the ARM, Multi-arch builds and helm charts that fall under control plane. ``` - openebs-archive/e2e-tests#405 - openebs#3038 - openebs#3023 - openebs#3037 - openebs#1295 - openebs/linux-utils#5 - openebs-archive/node-disk-manager#446 - openebs-archive/node-disk-manager#449 - openebs-archive/node-disk-manager#428 ``` - Peeyush Gupta for the contributions to the Power builds and control plane in general. ``` - openebs/charts#127 - openebs-archive/node-disk-manager#448 - openebs-archive/maya#1632 - openebs-archive/jiva#279 - openebs-archive/maya#667 - openebs-archive/istgt#131 - openebs-archive/maya#561 - openebs-archive/maya#750 ``` Signed-off-by: kmova <[email protected]>
Refer: [GOVERNANCE.md](https://github.com/openebs/openebs/blob/master/GOVERNANCE.md). The following community members have been helping with enhancing and testing of several areas of the OpenEBS project that broadly fall in the purview of the control plane. Based on their past contributions and their interest/commitment to contributing to OpenEBS, we take pride in adding them as reviewers to the OpenEBS project. The list of community members in alphabetical order are: - Mehran Kholdi for the contributions to the Local PV and control plane in general. ``` - #2975 - 40+ commits in https://github.com/openebs/rawfile-localpv ``` - Michael Fornaro for the contributions to the ARM, Multi-arch builds and helm charts that fall under control plane. ``` - openebs-archive/e2e-tests#405 - #3038 - #3023 - #3037 - #1295 - openebs/linux-utils#5 - openebs-archive/node-disk-manager#446 - openebs-archive/node-disk-manager#449 - openebs-archive/node-disk-manager#428 ``` - Peeyush Gupta for the contributions to the Power builds and control plane in general. ``` - openebs/charts#127 - openebs-archive/node-disk-manager#448 - openebs-archive/maya#1632 - openebs-archive/jiva#279 - openebs-archive/maya#667 - openebs-archive/istgt#131 - openebs-archive/maya#561 - openebs-archive/maya#750 ``` Signed-off-by: kmova <[email protected]>
Refer: [GOVERNANCE.md](https://github.com/openebs/openebs/blob/master/GOVERNANCE.md). The following community members have been helping with enhancing and testing of several areas of the OpenEBS project that broadly fall in the purview of the control plane. Based on their past contributions and their interest/commitment to contributing to OpenEBS, we take pride in adding them as reviewers to the OpenEBS project. The list of community members in alphabetical order are: - Mehran Kholdi for the contributions to the Local PV and control plane in general. ``` - openebs#2975 - 40+ commits in https://github.com/openebs/rawfile-localpv ``` - Michael Fornaro for the contributions to the ARM, Multi-arch builds and helm charts that fall under control plane. ``` - openebs-archive/e2e-tests#405 - openebs#3038 - openebs#3023 - openebs#3037 - openebs#1295 - openebs/linux-utils#5 - openebs-archive/node-disk-manager#446 - openebs-archive/node-disk-manager#449 - openebs-archive/node-disk-manager#428 ``` - Peeyush Gupta for the contributions to the Power builds and control plane in general. ``` - openebs/charts#127 - openebs-archive/node-disk-manager#448 - openebs-archive/maya#1632 - openebs-archive/jiva#279 - openebs-archive/maya#667 - openebs-archive/istgt#131 - openebs-archive/maya#561 - openebs-archive/maya#750 ``` Signed-off-by: kmova <[email protected]> Signed-off-by: Kung Fu Panda <[email protected]>
Fixes openebs/openebs#1922