ChainSafe / ChainSafe/gossamer

Define flags with their default values in newTestContext

Open
#1,894 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
454
Forks
144
PR merge metrics
No merged PRs in 30d

Description

## Issue summary

The piece of code described below is meant to define different cmd flags with their default values.
However, it defines them with actual value of the flag as its default value.
https://github.com/ChainSafe/gossamer/blob/bf903f2d03cbc72e224c1576300970e0702c39b3/cmd/gossamer/utils_test.go#L33-L46

Right after this is the code, that is meant to actually set the value. But because we have already used the actual values as default values, this codes ends up not having any impact.
https://github.com/ChainSafe/gossamer/blob/bf903f2d03cbc72e224c1576300970e0702c39b3/cmd/gossamer/utils_test.go#L50-L82

In general, this does not create any problems, but it did when I was trying to add a `StringSliceFlag`. This is because unlike other flags, StringSlice appends values to existing values when we do StringSlice.Set, while others would overwrite it.

What to change:-

A simpler way would be to use default values of each type like false for bool, 0 for int etc. These are not the true default values for the flag, but it would hurt since we are dealing with tests and all the values would be set later.

Other way is to have a map of flag name to default value and use that to set default values.

## Other information and links

-

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.