Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

test: add unit test case for func GetAllPeerIDs #1194

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

fenggw-fnst
Copy link
Contributor

Signed-off-by: Guangwen Feng fenggw-fnst@cn.fujitsu.com

Ⅰ. Describe what this PR did

Add unit test case for func GetAllPeerIDs

Ⅱ. Does this pull request fix one issue?

NONE

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

Ⅳ. Describe how to verify it

go test github.com/dragonflyoss/Dragonfly/supernode/daemon/mgr/peer -run Test -v

Ⅴ. Special notes for reviews


// get all peer ids
ids := manager.GetAllPeerIDs(context.Background())
c.Check(ids, check.DeepEquals, []string{id, id2})
Copy link
Contributor

Choose a reason for hiding this comment

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

CI fails here:

----------------------------------------------------------------------
FAIL: manager_test.go:130: PeerMgrTestSuite.TestGetAllPeerIDs

manager_test.go:157:
    c.Check(ids, check.DeepEquals, []string{id, id2})
... obtained []string = []string{"bar-192.168.10.11-1579514426165976773", "foo-192.168.10.11-1579514426165782207"}
... expected []string = []string{"foo-192.168.10.11-1579514426165782207", "bar-192.168.10.11-1579514426165976773"}

OOPS: 3 passed, 1 FAILED
--- FAIL: Test (0.01s)
FAIL
coverage: 86.2% of statements
FAIL	github.com/dragonflyoss/Dragonfly/supernode/daemon/mgr/peer	0.400s
make: *** [Makefile:109: unit-test] Error 1

Maybe you can change the code to sort the slice.

@allencloud
Copy link
Contributor

Any update? @fenggw-fnst

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1194      +/-   ##
==========================================
+ Coverage   47.72%   47.73%   +0.01%     
==========================================
  Files         115      115              
  Lines        7254     7254              
==========================================
+ Hits         3462     3463       +1     
+ Misses       3513     3511       -2     
- Partials      279      280       +1
Impacted Files Coverage Δ
supernode/daemon/mgr/scheduler/manager.go 21.91% <0%> (-0.69%) ⬇️
supernode/daemon/mgr/peer/manager.go 80.89% <0%> (+2.24%) ⬆️

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 919ff03...d1fbbd3. Read the comment docs.

@fenggw-fnst
Copy link
Contributor Author

Sorry for late reply. Done.
PTAL, thanks!

@allencloud
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Feb 17, 2020
@allencloud allencloud merged commit caad9fd into dragonflyoss:master Feb 17, 2020
@fenggw-fnst fenggw-fnst deleted the work2 branch March 3, 2020 12:24
sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022
Signed-off-by: Jim Ma <majinjing3@gmail.com>
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.

None yet

4 participants