Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

pkg/syncmap: add unit test for GetAsAtomicInt #1216

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

zhouhao3
Copy link
Contributor

Signed-off-by: Zhou Hao [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

Codecov Report

Merging #1216 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1216      +/-   ##
==========================================
+ Coverage   47.78%   47.84%   +0.06%     
==========================================
  Files         118      118              
  Lines        7436     7436              
==========================================
+ Hits         3553     3558       +5     
+ Misses       3596     3591       -5     
  Partials      287      287
Impacted Files Coverage Δ
supernode/daemon/mgr/scheduler/manager.go 22.6% <0%> (+0.68%) ⬆️
pkg/syncmap/syncmap.go 60.82% <0%> (+4.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 790e620...31003ca. Read the comment docs.

mmap.Add("aaa", expected)

result, _ := mmap.GetAsAtomicInt("aaa")
c.Check(result, check.DeepEquals, expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could add code here to check the non-existent one, then check the non-nil error:

result, err := mmap.GetAsAtomicInt("nonexist")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, PTAL

@zhouhao3 zhouhao3 force-pushed the add-suncmap-unit-test branch from 31003ca to f148faf Compare February 14, 2020 03:21
@allencloud
Copy link
Contributor

CI failed, PTAL:

./hack/golangci-lint.sh
Begin to check gocode with golangci-lint locally
Detected that golangci-lint has already been installed. Start linting...
pkg/syncmap/syncmap_test.go:145:2: SA4006: this value of `result` is never used (staticcheck)
	result, err := mmap.GetAsAtomicInt("nonexist")
	^
Makefile:160: recipe for target 'golangci-lint' failed
make: *** [golangci-lint] Error 1


result, err := mmap.GetAsAtomicInt("nonexist")
c.Check(err, check.NotNil)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add c.Check(result, check.IsNil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, thanks.

@zhouhao3 zhouhao3 force-pushed the add-suncmap-unit-test branch from f148faf to 11ce918 Compare February 14, 2020 03:28
@allencloud
Copy link
Contributor

LGTM

@allencloud allencloud merged commit a392463 into dragonflyoss:master Feb 14, 2020
@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Feb 14, 2020
@zhouhao3 zhouhao3 deleted the add-suncmap-unit-test branch February 14, 2020 04:50
sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
areas/test LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants