[ioctl] Cleanup TestNewNodeDelegateCmd #3421

LuckyPigeon · 2022-05-30T07:13:50Z

Description

Cleanup TestNewNodeDelegateCmd with following adjustment.

Remove redundant mock test
Replace AnyTimes with specific Times
Move specific client and variable to correspond t.Run

Type of change

Please delete options that are not relevant.

Code refactor or improvement

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

TestNewNodeDelegateCmd

Test Configuration:

Firmware version: Ubuntu 21.04
Hardware:
Toolchain:
SDK:

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

codecov · 2022-05-30T07:31:09Z

Codecov Report

Merging #3421 (c61ae2d) into master (e3afa71) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #3421      +/-   ##
==========================================
- Coverage   75.28%   75.27%   -0.02%     
==========================================
  Files         241      241              
  Lines       22258    22261       +3     
==========================================
- Hits        16758    16757       -1     
- Misses       4597     4602       +5     
+ Partials      903      902       -1

Impacted Files	Coverage Δ
ioctl/newcmd/node/nodedelegate.go	`71.71% <50.00%> (-0.78%)`	⬇️
db/trie/mptrie/branchnode.go	`97.22% <0.00%> (-1.39%)`	⬇️

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 e3afa71...c61ae2d. Read the comment docs.

ioctl/newcmd/node/nodedelegate_test.go

huof6829 · 2022-06-02T13:46:13Z

nodedelegate.go#L99 bc.GetProbationList(client, epochNum) should add epochStartHeight. see

iotex-core/ioctl/cmd/node/nodedelegate.go

Line 290 in 46602a9

probationListRes, err := bc.GetProbationList(_epochNum, epochStartHeight)

huof6829 · 2022-06-02T13:50:48Z

nodedelegate.go#L99 var err error can be delete

LuckyPigeon · 2022-06-10T12:15:48Z

I have added epochStartHeight at #3411 once, but @Liuhaai tells me to revert, and I'm not sure we need to add it or not.
If it's a necessary, I'll add it both here and #3411.

LuckyPigeon · 2022-06-10T12:20:31Z

var err error in nodedelegate.go #L102 had been used at

iotex-core/ioctl/newcmd/node/nodedelegate.go

Lines 240 to 243 in 623be41

    
           if err != nil { 
        
           	cmd.Println(err) 
        
           } 
        
           return nil

huof6829 · 2022-06-13T07:50:37Z

I have added epochStartHeight at #3411 once, but @Liuhaai tells me to revert, and I'm not sure we need to add it or not. If it's a necessary, I'll add it both here and #3411.

I'v modified #3411. After #3411 merged, you can use getProbationList() for line 206~215

huof6829 · 2022-06-02T14:11:46Z

ioctl/newcmd/node/nodedelegate.go

@@ -244,8 +247,6 @@ func NewNodeDelegateCmd(client ioctl.Client) *cobra.Command {
 		flagEpochNumUsage)
 	cmd.Flags().BoolVarP(&nextEpoch, "next-epoch", "n", false,


@CoderZhi next-epoch is a new flag here and not exist in ioctl/cmd/node/nodedelegate.go. I'm not sure if it can be used like this. In cmd nextEpoch is distinguished by

iotex-core/ioctl/cmd/node/nodedelegate.go

Lines 151 to 153 in 46602a9

if epochData.Height >= config.ReadConfig.Nsv2height {

return delegatesV2(probationList, response, &message)

}

I think we should create new pr to change to the original logic.

huof6829 · 2022-06-13T08:02:24Z

var err error in nodedelegate.go #L102 had been used at

iotex-core/ioctl/newcmd/node/nodedelegate.go

Lines 240 to 243 in 623be41

if err != nil {

cmd.Println(err)

}

return nil

fixed

ioctl/newcmd/node/nodedelegate.go

Liuhaai · 2022-06-14T16:27:02Z

map could be replaced with if... else

ioctl/newcmd/node/nodedelegate.go

* upstream/master: (24 commits) account type with zero init nonce (iotexproject#3387) [api] Separate Server and Server Handler (iotexproject#3485) [ioctl] Build hdwallet derive command line into new ioctl (iotexproject#3418) [ioctl] Build hdwallet create command line into new ioctl (iotexproject#3470) [makefile] add go mod tidy (iotexproject#3471) [api] update chain metrics (iotexproject#3484) remove config.EVMNetworkID() (iotexproject#3460) [filedao] remove checkMasterChainDBFile() (iotexproject#3463) [api] add crashlog (iotexproject#3456) [api] Move generateBlockMeta to grpcserver.go (iotexproject#3303) [ioctl] Build action hash command line into new ioctl (iotexproject#3425) [ioctl] Build hdwallet export command line into new ioctl (iotexproject#3423) [ioctl] Refactor nodereward command in new ioctl (iotexproject#3416) [ioctl] Cleanup TestNewNodeDelegateCmd (iotexproject#3421) [blockchain] Remove BoltDBDaoOption (iotexproject#3465) remove InMemDaoOption (iotexproject#3464) [action] add evm london test (iotexproject#3402) [ioctl] create main for ioctl/newcmd (iotexproject#3296) [ioctl] Build block bucket command line into new ioctl (iotexproject#3386) [ioctl] Build hdwallet import command line into new ioctl (iotexproject#3419) ...

[ioctl] cleanup TestNewNodeDelegateCmd

a7c3ea9

LuckyPigeon marked this pull request as ready for review May 30, 2022 08:44

LuckyPigeon requested a review from a team as a code owner May 30, 2022 08:44

huof6829 reviewed May 31, 2022

View reviewed changes

ioctl/newcmd/node/nodedelegate_test.go Outdated Show resolved Hide resolved

ioctl/newcmd/node/nodedelegate_test.go Outdated Show resolved Hide resolved

ioctl/newcmd/node/nodedelegate_test.go Outdated Show resolved Hide resolved

LuckyPigeon and others added 4 commits June 1, 2022 12:07

Merge branch 'master' into dev_node_delegate_cleanup

d8c3b8c

check false active block

f08cb1b

check false active delegate

e5e2243

Merge branch 'master' into dev_node_delegate_cleanup

a3890e6

Merge branch 'master' into dev_node_delegate_cleanup

623be41

huof6829 closed this Jun 13, 2022

huof6829 reopened this Jun 13, 2022

delete useless err

73f5cc5

huof6829 reviewed Jun 13, 2022

View reviewed changes

huof6829 approved these changes Jun 13, 2022

View reviewed changes

Liuhaai reviewed Jun 13, 2022

View reviewed changes

ioctl/newcmd/node/nodedelegate.go Outdated Show resolved Hide resolved

LuckyPigeon added 2 commits June 14, 2022 17:44

Merge branch 'master' into dev_node_delegate_cleanup

dee5d55

add subfunc to replace map design

5c4b2bd

LuckyPigeon added 2 commits June 18, 2022 16:01

Merge branch 'master' into dev_node_delegate_cleanup

fbd6d8a

replace map with if...else statement

43d413c

Liuhaai reviewed Jun 20, 2022

View reviewed changes

ioctl/newcmd/node/nodedelegate.go Outdated Show resolved Hide resolved

Liuhaai reviewed Jun 20, 2022

View reviewed changes

ioctl/newcmd/node/nodedelegate.go Outdated Show resolved Hide resolved

Liuhaai reviewed Jun 20, 2022

View reviewed changes

ioctl/newcmd/node/nodedelegate.go Outdated Show resolved Hide resolved

LuckyPigeon and others added 4 commits June 23, 2022 09:59

Merge branch 'master' into dev_node_delegate_cleanup

f57fa2d

remove ConfigMap

41f020c

Merge branch 'master' into dev_node_delegate_cleanup

ec3cf13

Merge branch 'master' into dev_node_delegate_cleanup

c61ae2d

CoderZhi approved these changes Jun 25, 2022

View reviewed changes

CoderZhi merged commit bd7662c into iotexproject:master Jun 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ioctl] Cleanup TestNewNodeDelegateCmd #3421

[ioctl] Cleanup TestNewNodeDelegateCmd #3421

LuckyPigeon commented May 30, 2022 •

edited

Loading

codecov bot commented May 30, 2022 •

edited

Loading

huof6829 commented Jun 2, 2022 •

edited

Loading

huof6829 commented Jun 2, 2022

LuckyPigeon commented Jun 10, 2022

LuckyPigeon commented Jun 10, 2022 •

edited

Loading

huof6829 commented Jun 13, 2022 •

edited

Loading

huof6829 Jun 2, 2022

huof6829 commented Jun 13, 2022

Liuhaai commented Jun 14, 2022 •

edited

Loading

		@@ -244,8 +247,6 @@ func NewNodeDelegateCmd(client ioctl.Client) *cobra.Command {
		flagEpochNumUsage)
		cmd.Flags().BoolVarP(&nextEpoch, "next-epoch", "n", false,

	if epochData.Height >= config.ReadConfig.Nsv2height {
	return delegatesV2(probationList, response, &message)
	}

[ioctl] Cleanup TestNewNodeDelegateCmd #3421

[ioctl] Cleanup TestNewNodeDelegateCmd #3421

Conversation

LuckyPigeon commented May 30, 2022 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

codecov bot commented May 30, 2022 • edited Loading

Codecov Report

huof6829 commented Jun 2, 2022 • edited Loading

huof6829 commented Jun 2, 2022

LuckyPigeon commented Jun 10, 2022

LuckyPigeon commented Jun 10, 2022 • edited Loading

huof6829 commented Jun 13, 2022 • edited Loading

huof6829 Jun 2, 2022

Choose a reason for hiding this comment

huof6829 commented Jun 13, 2022

Liuhaai commented Jun 14, 2022 • edited Loading

LuckyPigeon commented May 30, 2022 •

edited

Loading

codecov bot commented May 30, 2022 •

edited

Loading

huof6829 commented Jun 2, 2022 •

edited

Loading

LuckyPigeon commented Jun 10, 2022 •

edited

Loading

huof6829 commented Jun 13, 2022 •

edited

Loading

Liuhaai commented Jun 14, 2022 •

edited

Loading