test: added tests for the cmd-filters package #892
Conversation
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
from
2d3afd1
to
cfa05a7
Compare
VaibhavMalik4187
changed the title
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
2 times, most recently
from
98dfa72
to
43531d6
Compare
|
@AlexsJones, I've tried to add some tests to the cmd-filters package and a minor modification in the filters list command. Could you please go through this PR whenever you have a moment? Thanks! |
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
from
43531d6
to
3ed5844
Compare
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
2 times, most recently
from
c4dccb1
to
2fd1c46
Compare
|
@AlexsJones, I've fixed the CI errors. |
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
3 times, most recently
from
ea579a7
to
1a473e7
Compare
cmd/filters/add_test.go
Outdated
| err := addCmd.Args(&cobra.Command{}, []string{"arg1"}) | ||
| require.NoError(t, err) | ||
|
|
||
| err = addCmd.Args(&cobra.Command{}, []string{"arg1", "arg2"}) |
There was a problem hiding this comment.
what are these arg1, arg2 values? what will actual values for these?
There was a problem hiding this comment.
I thought about it and realized that these are useless, hence, I've got rid of them in the latest version.
|
|
||
| // Initially the list contained 12 filters, after deleting one filter in this | ||
| // test, now it should be left with 11 filters only. | ||
| require.Equal(t, 11, len(viper.GetStringSlice("active_filters"))) |
There was a problem hiding this comment.
how about use length of data[active_filters]. it's more safe later
There was a problem hiding this comment.
data is not up to date with the active filters, its sole purpose was to populate the configuration file.
There was a problem hiding this comment.
i see. but we can do this. len(data['active_filters']) - 1 since we remove one filter
cmd/filters/list_test.go
Outdated
| got, _ := io.ReadAll(r) | ||
| os.Stdout = rescueStdout | ||
|
|
||
| want := "Active: \n> CronJob\n> Deployment\n> HorizontalPodAutoScaler\n> Ingress\n> MutatingWebhookConfiguration\n> Node\n> PersistentVolumeClaim\n> Pod\n> ReplicaSet\n> Service\n> StatefulSet\n> ValidatingWebhookConfiguration\nUnused: \n> Gateway\n> GatewayClass\n> HTTPRoute\n> Log\n> NetworkPolicy\n> PodDisruptionBudget\n" |
There was a problem hiding this comment.
are these default filters?
if so, default filters can vary in the future.
There was a problem hiding this comment.
Yes, this test is not very useful. I'll remove it soon.
This commit adds new tests for the `cmd/filters` package. As a result, the code coverage of the package has increased from 0% to almost 51% This also includes a minor adjustment in the list command. Now, the list of the filters will always be in sorted order for better consistency. Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>
VaibhavMalik4187
force-pushed
the
cmd-filters-tests
branch
from
1a473e7
to
5b8ef5d
Compare
| // Set the configuration file in viper | ||
| configFileName := "delete-config.json" | ||
| data := map[string]interface{}{ | ||
| "active_filters": []string{ |
There was a problem hiding this comment.
this has 11 filters not 12
📑 Description
This commit adds new tests for the
cmd/filterspackage. As a result,the code coverage of the package has increased from 0% to almost 75%
This also includes a minor adjustment in the list command. Now, the list
of the filters will always be in sorted order for better consistency.
✅ Checks