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

pkg/utils: add utils_test #736

Merged
merged 1 commit into from Jan 8, 2020

Conversation

zhouhao3
Copy link

Add corresponding unit tests to the functions in util.go.

Signed-off-by: Zhou Hao zhouhao@cn.fujitsu.com


for _, tt := range testData {
err := ValidateContainerID(tt.containerID)
if err != nil && !tt.err {
Copy link
Member

Choose a reason for hiding this comment

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

Could you help to use DeepEqual to squash these lines?

Copy link
Author

Choose a reason for hiding this comment

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

They are different structure types, and it seems that it is not suitable to use DeepEqual. If they are changed to agree types and then compared, more code will be generated.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

updated, thanks for your suggestions. PTAL

@mars1024
Copy link
Member

BTW, commits need to be squashed either.

@zhouhao3 zhouhao3 force-pushed the add-utils-test branch 2 times, most recently from 35cc4d9 to 025e32f Compare December 27, 2019 03:32
Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com>
@dcbw
Copy link
Member

dcbw commented Jan 8, 2020

/lgtm

@dcbw dcbw merged commit a09ec7a into containernetworking:master Jan 8, 2020
@zhouhao3 zhouhao3 deleted the add-utils-test branch January 9, 2020 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants