Skip to content

Increase unit coverage#1685

Open
craigell wants to merge 2 commits into
dev-v2from
increase-unit-coverage
Open

Increase unit coverage#1685
craigell wants to merge 2 commits into
dev-v2from
increase-unit-coverage

Conversation

@craigell
Copy link
Copy Markdown
Contributor

@craigell craigell commented May 14, 2026

Proposed changes

Increased unit test coverage for agent v2.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

@craigell craigell requested a review from a team as a code owner May 14, 2026 11:40
@github-actions github-actions Bot added chore Pull requests for routine tasks documentation Improvements or additions to documentation labels May 14, 2026
@craigell craigell added the v2.x Issues and Pull Requests related to the major version v2 label May 14, 2026
@craigell craigell self-assigned this May 14, 2026
Comment thread sdk/traverser_test.go
return &crossplane.Config{Parsed: []*crossplane.Directive{events, http}}
}

func TestCrossplaneConfigTraverse_VisitsEveryDirective(t *testing.T) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the test's be grouped and use structs for differenet inputs and expected outputs like here in this file

want []string
}{
{
name: "empty map returns empty slice",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a number to the name of the tests is possible

port int
expect bool
}{
{"both set", "localhost", 1234, true},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be edited to be made more clear the value being set like the above test

Suggested change
{"both set", "localhost", 1234, true},
{
name: "both set",
host: "localhost",
port: 1234,
expectL true
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Pull requests for routine tasks documentation Improvements or additions to documentation v2.x Issues and Pull Requests related to the major version v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants