Task: Get config cmd #3552

pocockn · 2022-07-15T10:26:49Z

Description

Step 1: refactor command _configGetCmd into ./ioctl/newcmd/config/config_get.go
Step 2: create ./ioctl/newcmd/config/config_get_test.go to add unit test

Fixes #3467

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

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

make test

* 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) ...

huof6829 · 2022-07-15T11:54:22Z

@pocockn Thanks!

huof6829 · 2022-07-15T11:55:13Z

Pls fix ci fail, thx.

huof6829 · 2022-07-15T12:21:28Z

ioctl/newcmd/config/config_get.go

+	long, _ := client.SelectTranslation(_configGetUseCmdLong)
+
+	return &cobra.Command{
+		Use:       "VARIABLE",


"get VARIABLE" , add translate get 变量

huof6829 · 2022-07-15T12:33:04Z

ioctl/newcmd/config/config.go

+}
+
+func (m *endpointMessage) String() string {
+	if output.Format == "" {


delete output package

huof6829 · 2022-07-15T12:37:37Z

ioctl/newcmd/config/config.go

+
+func (m *endpointMessage) String() string {
+	if output.Format == "" {
+		message := fmt.Sprint(m.Endpoint, "    secure connect(TLS):", m.SecureConnect)


directly move to line142

huof6829 · 2022-07-15T12:54:18Z

ioctl/newcmd/config/config_test.go

+	defer func() {
+		testutil.CleanupPath(testPath)
+	}()
+	_configDir = testPath


_configDir = os.TempDir()

huof6829 · 2022-07-15T12:54:35Z

ioctl/newcmd/config/config_test.go

+	defer func() {
+		testutil.CleanupPath(testPath)
+	}()
+	_configDir = testPath


_configDir = os.TempDir()

huof6829 · 2022-07-15T13:04:34Z

ioctl/newcmd/config/config_test.go

+	for _, tc := range tcs {
+		cfgItem, err := info.get(tc.arg)
+		require.NoError(err)
+		require.Contains(cfgItem, tc.expected)


expected data should be set before checking if no error

Would it be worth implementing the config set PR before this to use that functionality here?

@huof6829 :)

no need, here just test function get()

huof6829 · 2022-07-15T13:11:46Z

ioctl/newcmd/config/config_test.go

+	cfg, cfgFilePath, err := InitConfig()
+	info := newInfo(cfg, cfgFilePath)
+
+	tcs := []struct {


combine into line50
if err!=nil {require.Contains(err.Error() }

huof6829 · 2022-07-15T13:12:15Z

ioctl/newcmd/config/config_test.go

+	}
+}
+
+func TestConfigGetError(t *testing.T) {


huof6829 · 2022-07-15T13:14:26Z

ioctl/newcmd/config/config.go

+// get retrieves a config item from its key.
+func (c *info) get(arg string) (string, error) {
+	switch arg {
+	default:


move to end

…witch, improve test setup

…task/config-get * 'task/config-get' of github.com:pocockn/iotex-core: [ioctl] Build action command line into new ioctl (iotexproject#3472) fix potential file inclusion via variable (iotexproject#3549)

codecov · 2022-07-16T05:54:54Z

Codecov Report

Merging #3552 (dc6c903) into master (a20e489) will decrease coverage by 1.34%.
The diff coverage is 51.50%.

@@            Coverage Diff             @@
##           master    #3552      +/-   ##
==========================================
- Coverage   75.43%   74.09%   -1.35%     
==========================================
  Files         247      254       +7     
  Lines       22845    23385     +540     
==========================================
+ Hits        17233    17326      +93     
- Misses       4685     5133     +448     
+ Partials      927      926       -1

Impacted Files	Coverage Δ
action/protocol/staking/read_state.go	`15.38% <0.00%> (ø)`
api/grpcserver.go	`79.49% <ø> (+0.55%)`	⬆️
blockchain/blockchain.go	`0.89% <0.00%> (ø)`
blockchain/filedao/filedao_legacy.go	`85.80% <ø> (ø)`
blockchain/filedao/filedao_v2.go	`98.51% <ø> (ø)`
blockchain/pubsubmanager.go	`0.00% <0.00%> (ø)`
config/config.go	`94.06% <ø> (+9.31%)`	⬆️
consensus/scheme/rolldpos/rolldposctx.go	`73.47% <0.00%> (-0.18%)`	⬇️
db/kvstore_impl.go	`61.64% <ø> (ø)`
db/kvstorewithcache.go	`0.00% <0.00%> (ø)`
... and 59 more

huof6829 · 2022-07-19T08:54:25Z

@pocockn I set endpoint="", so line85 require.Contains(err.Error(), tc.expected) can be checked. InitConfig is done by TestInitConfig, so directly use test datas to check.

pocockn · 2022-07-19T10:08:13Z

@pocockn I set endpoint="", so line85 require.Contains(err.Error(), tc.expected) can be checked. InitConfig is done by TestInitConfig, so directly use test datas to check.

I see what you mean now, thank you! Are there any more changes?

huof6829 · 2022-07-19T11:52:15Z

ioctl/newcmd/config/config.go

+	case "analyserEndpoint":
+		return c.readConfig.AnalyserEndpoint, nil
+	case "all":
+		return c.readConfig.String(), nil


line146 and line156 calls output's function.

iotex-core/ioctl/output/format.go

Line 180 in 2ed5b03

func JSONString(out interface{}) string {

directly refactor it here and call. don't use output

Thanks for your help, done.

…task/config-get * 'task/config-get' of github.com:pocockn/iotex-core: modify test move chanid metrics to chainservice (iotexproject#3544) [ioctl] fix log entries created from user input (iotexproject#3546) add log in rolldposctx (iotexproject#3553) fix uncontrolled data used in path expression (iotexproject#3547) [api] impl. TestGrpcServer_GetServerMeta (iotexproject#3559)

huof6829 · 2022-07-20T02:23:47Z

ioctl/newcmd/config/config.go

+		message := endpointMessage{Endpoint: c.readConfig.Endpoint, SecureConnect: c.readConfig.SecureConnect}
+		return fmt.Sprint(message.Endpoint, "    secure connect(TLS):", message.SecureConnect), nil


delete endpointMessage, directly use value in line141

huof6829 · 2022-07-20T02:28:28Z

ioctl/newcmd/config/config.go

+	}
+}
+
+type endpointMessage struct {


huof6829 · 2022-07-20T03:51:18Z

ioctl/newcmd/config/config.go

+func jsonString(out interface{}) string {
+	byteAsJSON, err := json.MarshalIndent(out, "", "  ")
+	if err != nil {
+		log.Panic(err)


…task/config-get * 'task/config-get' of github.com:pocockn/iotex-core: [staking] unexport namespace (iotexproject#3551)

huof6829 · 2022-07-20T09:03:57Z

@pocockn Thanks for your help!

Liuhaai · 2022-07-21T03:48:50Z

ioctl/newcmd/config/config_get.go

+		Short:     short,
+		Long:      long,
+		ValidArgs: _validGetArgs,
+		Args: func(cmd *cobra.Command, args []string) error {


@huof6829 pls help fix args issue

fixed. use cobra.ExactValidArgs(1)

* upstream/master: (45 commits) Task: Get config cmd (iotexproject#3552) [ioctl] fix Errors unhandled (iotexproject#3567) fix dir permission and file inclusion (iotexproject#3566) [test] Disable workingset cache in the benchmark test (iotexproject#3558) [pkg] fix deferring unsafe method "Close" on type "*os.File" (iotexproject#3548) [action] Refactor handleTransfer() (iotexproject#3557) Add MinVersion in tls.Config (iotexproject#3562) [ioctl] Modify file permission as 0600 (iotexproject#3563) [httputil] add ReadHeaderTimeout (iotexproject#3550) [staking] unexport namespace (iotexproject#3551) move chanid metrics to chainservice (iotexproject#3544) [ioctl] fix log entries created from user input (iotexproject#3546) add log in rolldposctx (iotexproject#3553) fix uncontrolled data used in path expression (iotexproject#3547) [api] impl. TestGrpcServer_GetServerMeta (iotexproject#3559) [ioctl] Build action command line into new ioctl (iotexproject#3472) fix potential file inclusion via variable (iotexproject#3549) [ioctl] Incorrect conversion between integer types (iotexproject#3522) [action] fix incorrect conversion between integer types (iotexproject#3545) [test] fix TestLoadBlockchainfromDB (iotexproject#3521) ...

pocockn and others added 8 commits June 9, 2022 15:18

Task: Return error from keep alive methods within httputil pkg

996573e

Merge branch 'master' into master

3b3504e

Merge branch 'master' into master

e705bba

Merge branch 'master' into master

3a272b4

Initial commit for get config task

6c0e227

Add unit tests

c7a0f7c

Add config get command test

f1b42f9

pocockn requested a review from a team as a code owner July 15, 2022 10:26

huof6829 reviewed Jul 15, 2022

View reviewed changes

pocockn added 3 commits July 16, 2022 07:41

Merge branch 'master' into task/config-get

e301d11

Remove output package, change case default to last statement within s…

e7b9f69

…witch, improve test setup

Merge branch 'task/config-get' of github.com:pocockn/iotex-core into …

f4535ad

…task/config-get * 'task/config-get' of github.com:pocockn/iotex-core: [ioctl] Build action command line into new ioctl (iotexproject#3472) fix potential file inclusion via variable (iotexproject#3549)

modify test

05c7def

Merge branch 'master' into task/config-get

ad7588e

huof6829 reviewed Jul 19, 2022

View reviewed changes

pocockn added 4 commits July 19, 2022 15:51

Add jsonString method

1baceb3

make json string unexported

d678d3d

Merge branch 'master' into task/config-get

76032ef

huof6829 reviewed Jul 20, 2022

View reviewed changes

pocockn added 4 commits July 20, 2022 08:14

Remove message struct and return endpoint data directly

4e8e15f

Merge branch 'task/config-get' of github.com:pocockn/iotex-core into …

8d406aa

…task/config-get * 'task/config-get' of github.com:pocockn/iotex-core: [staking] unexport namespace (iotexproject#3551)

Add wrapped erorr

9b6fbc1

Merge branch 'master' into task/config-get

a62ef54

huof6829 approved these changes Jul 20, 2022

View reviewed changes

huof6829 requested a review from Liuhaai July 20, 2022 09:02

Merge branch 'master' into task/config-get

fe42fa7

Liuhaai approved these changes Jul 21, 2022

View reviewed changes

Liuhaai reviewed Jul 21, 2022

View reviewed changes

huof6829 added 2 commits July 21, 2022 13:50

use ExactValidArgs()

62cc50f

Merge branch 'master' into task/config-get

dc6c903

huof6829 merged commit 23c6de5 into iotexproject:master Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task: Get config cmd #3552

Task: Get config cmd #3552

pocockn commented Jul 15, 2022 •

edited

Loading

huof6829 commented Jul 15, 2022

huof6829 commented Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

pocockn Jul 16, 2022

pocockn Jul 18, 2022

huof6829 Jul 19, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

huof6829 Jul 15, 2022

codecov bot commented Jul 16, 2022 •

edited

Loading

huof6829 commented Jul 19, 2022 •

edited

Loading

pocockn commented Jul 19, 2022

huof6829 Jul 19, 2022

pocockn Jul 19, 2022

huof6829 Jul 20, 2022

huof6829 Jul 20, 2022

huof6829 Jul 20, 2022

huof6829 commented Jul 20, 2022

Liuhaai Jul 21, 2022 •

edited

Loading

huof6829 Jul 21, 2022

		message := endpointMessage{Endpoint: c.readConfig.Endpoint, SecureConnect: c.readConfig.SecureConnect}
		return fmt.Sprint(message.Endpoint, " secure connect(TLS):", message.SecureConnect), nil

Task: Get config cmd #3552

Task: Get config cmd #3552

Conversation

pocockn commented Jul 15, 2022 • edited Loading

Description

Type of change

How Has This Been Tested?

huof6829 commented Jul 15, 2022

huof6829 commented Jul 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 16, 2022 • edited Loading

Codecov Report

huof6829 commented Jul 19, 2022 • edited Loading

pocockn commented Jul 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huof6829 commented Jul 20, 2022

Liuhaai Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pocockn commented Jul 15, 2022 •

edited

Loading

codecov bot commented Jul 16, 2022 •

edited

Loading

huof6829 commented Jul 19, 2022 •

edited

Loading

Liuhaai Jul 21, 2022 •

edited

Loading