chore: add cobra commands validator #767

ankithans · 2021-06-30T11:02:07Z

Description

Add charmil validator in root_test.go, for testing of cobra commands. The validator has some default rules defined, that can be overridden.
Currently Validator supports 2 rules:

Length Rule(ensures that cobra.Command attribute(like Use, Short) have defined length of characters)
Must Exist(ensures the field is present)

logs

$ go test ./pkg/cmd/root/
commands checked: 35
checks failed: 47
--- FAIL: Test_ExecuteCommand (0.01s)
    root_test.go:40: LENGTH_RULE: cmd rhoas: Short length should be at least 15
    root_test.go:40: LENGTH_RULE: cmd rhoas: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas cluster connect: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas cluster status: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas completion bash: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas completion bash: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas completion fish: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas completion fish: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas completion zsh: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas completion zsh: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup delete: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup delete: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup describe: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup describe: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup list: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic create: Short length should be at least 15
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic create: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic delete: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic delete: Short length should be at least 15
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic delete: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic describe: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic describe: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic list: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic update: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka consumergroup: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas kafka consumergroup: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka list: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas kafka list: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka topic: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas kafka topic: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas serviceaccount delete: Example length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas cluster: Long length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas cluster: Long must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas completion: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas completion: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas kafka: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas kafka: Long must be present
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas kafka: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas logout: Long length should be at least 100
    root_test.go:40: LENGTH_RULE: cmd rhoas logout: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas logout: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas serviceaccount: Example length should be at least 100
    root_test.go:40: MUST_EXIST_RULE: cmd rhoas serviceaccount: Example must be present
    root_test.go:40: LENGTH_RULE: cmd rhoas whoami: Example length should be at least 100
FAIL
FAIL    github.com/redhat-developer/app-services-cli/pkg/cmd/root       0.023s
FAIL

Verification Steps

Run go get github.com/aerogear/charmil
Run go test ./pkg/cmd/root

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation change
Other (please specify)

Checklist

Documentation added for the feature
CI and all relevant tests are passing
Code Review completed
Verified independently by reviewer

ankithans · 2021-06-30T11:02:38Z

cc: @wtrocki

wtrocki · 2021-06-30T11:10:18Z

@ankithans I think we need to adjust our rules for CLI to pass. Then we can create issue for rhoas to fix those problems.

This will require alignment with documentation teams and @craicoverflow as lead, but overall we show value of having validator for commands.

Let's create issue with errors for ideal rules we want to have and adjust rules now to have it pass.

wtrocki · 2021-06-30T11:10:38Z

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/BurntSushi/toml v0.3.1
 	github.com/MakeNowJust/heredoc v1.0.0
 	github.com/Nerzal/gocloak/v7 v7.11.0
+	github.com/aerogear/charmil v0.0.0-20210630101651-4ba3da6c5dbc


For merge we will need official tag/release

Lets do release :)

wtrocki · 2021-06-30T11:11:41Z

pkg/cmd/root/root_test.go

@@ -0,0 +1,104 @@
+package root


This is not great for visibility.

We can call it:

charmil/rules

cobralint

etc.

We can also keep that in the root but then rename Test_ExecuteCommand to be more descriptive

wtrocki · 2021-06-30T11:14:56Z

pkg/cmd/root/root_test.go

+	var vali rules.RuleConfig
+	validationErr := vali.ExecuteRules(cmd)
+	for _, errs := range validationErr {
+		if errs.Err != nil {


We should fail test if error happends.

wtrocki · 2021-06-30T11:16:18Z

pkg/cmd/root/root_test.go

+		}
+	}
+
+	if len(validationErr) == 0 {


Actually I have idea. How about we do keep test passing but have commented out assertions and follow up issue.
That will be better than changing rules. If we can make rules more adjusted to existing commands in rhoas + keep asertions to detect new commands but ignore errors that we currently have tat will be great deal.

craicoverflow · 2021-06-30T13:10:10Z

I think we need to adjust our rules for CLI to pass. Then we can create issue for rhoas to fix those problems.

Will there be a .charmillint.yaml or similar to adjust the rules based on this project?

feat: add cobra commands validator

Before merging ensure that the conventional commit type is different (a chore or test)

ankithans · 2021-06-30T13:27:43Z

Will there be a .charmillint.yaml or similar to adjust the rules based on this project?

for now configurations can be done in the RuleConfig object itself.

var vali rules.RuleConfig = rules.RuleConfig{
	Verbose: true,
	MustExist: rules.MustExist{
		Fields: []string{"Args"},
	},
}

We had a talk here (aerogear/charmil#89 (comment)) and decided that having config file will work great for similar CLI based solution, but not for a testing framework. But this can be reconsidered according to the requirements!

Before merging ensure that the conventional commit type is different (a chore or test)

sure 👍

wtrocki · 2021-06-30T16:21:01Z

Rebase needed.

ankithans · 2021-06-30T16:35:20Z

@wtrocki rebase done

wtrocki · 2021-06-30T17:12:46Z

pkg/cmd/root/root_test.go

+	"github.com/redhat-developer/app-services-cli/pkg/localize/goi18n"
+)
+
+func Test_ExecuteCommand(t *testing.T) {


wtrocki · 2021-06-30T17:13:38Z

pkg/cmd/root/root_test.go

+	}
+}
+
+func initConfig(f *factory.Factory) {


That is this for? We have mockutil package that does mock config

wtrocki · 2021-07-01T11:27:10Z

pkg/cmd/root/root_test.go

@@ -0,0 +1,47 @@
+package root


Validator_test

changed filename

wtrocki · 2021-07-01T11:27:21Z

pkg/cmd/root/root_test.go

+	"github.com/redhat-developer/app-services-cli/pkg/localize/goi18n"
+)
+
+func Test_RootRhoasCommand(t *testing.T) {


ValidateCommandsUsingCharmilValidator

Test_ValidateCommandsUsingCharmilValidator
did this, to run the test

wtrocki · 2021-07-01T11:29:15Z

Please put all errors that are happening into new issue and add info how to enable unit tests after they are fixed:

https://github.com/redhat-developer/app-services-cli/issues/new?assignees=&labels=bug&template=bug_report.md&title=

wtrocki reviewed Jun 30, 2021

View reviewed changes

ankithans changed the title ~~feat: add cobra commands validator~~ chore: add cobra commands validator Jun 30, 2021

wtrocki reviewed Jun 30, 2021

View reviewed changes

ankithans requested a review from wtrocki June 30, 2021 17:25

wtrocki reviewed Jul 1, 2021

View reviewed changes

ankithans added 2 commits July 1, 2021 18:27

chore: cobra cmd validation test in root

c0e14f3

chore: address review comments

e506b1e

wtrocki merged commit 725c5c2 into redhat-developer:main Jul 1, 2021

ankithans deleted the cobra-validator branch July 1, 2021 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add cobra commands validator #767

chore: add cobra commands validator #767

ankithans commented Jun 30, 2021

ankithans commented Jun 30, 2021

wtrocki commented Jun 30, 2021

wtrocki Jun 30, 2021

wtrocki Jun 30, 2021

wtrocki Jun 30, 2021 •

edited

Loading

wtrocki Jun 30, 2021

wtrocki Jun 30, 2021

craicoverflow commented Jun 30, 2021

ankithans commented Jun 30, 2021

wtrocki commented Jun 30, 2021

ankithans commented Jun 30, 2021

wtrocki Jun 30, 2021

wtrocki Jun 30, 2021

wtrocki Jul 1, 2021

ankithans Jul 1, 2021

wtrocki Jul 1, 2021

ankithans Jul 1, 2021

wtrocki commented Jul 1, 2021

chore: add cobra commands validator #767

chore: add cobra commands validator #767

Conversation

ankithans commented Jun 30, 2021

Description

logs

Verification Steps

Type of change

Checklist

ankithans commented Jun 30, 2021

wtrocki commented Jun 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wtrocki Jun 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craicoverflow commented Jun 30, 2021

ankithans commented Jun 30, 2021

wtrocki commented Jun 30, 2021

ankithans commented Jun 30, 2021

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

wtrocki commented Jul 1, 2021

wtrocki Jun 30, 2021 •

edited

Loading