Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add unit tests #15

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open

[WIP] Add unit tests #15

wants to merge 21 commits into from

Conversation

pawalt
Copy link

@pawalt pawalt commented Jan 3, 2020

This PR resolves #4

Describe your changes
This PR adds unit tests to common and fixes warnings brought up by go-lint

Testing performed
This is it

Additional context
This PR has a lot of small changes to variable naming and such. While we could keep our old names, I'd like to conform to go-lint as much as possible.
Also, this needs to wait until #14 is merged

@pawalt pawalt requested a review from dennisgove January 6, 2020 15:39
@pawalt pawalt force-pushed the pawalt/add-unit-tests branch 2 times, most recently from cd9852b to 0355290 Compare January 8, 2020 14:59
Peyton Walters and others added 12 commits January 8, 2020 17:54
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
internal/common/common_test.go Outdated Show resolved Hide resolved
cmd/plugin/vault-auth-spire.go Outdated Show resolved Hide resolved
internal/common/filetrustsource.go Outdated Show resolved Hide resolved
internal/common/settings.go Outdated Show resolved Hide resolved
if strings.Contains(domain, "/") {
return fmt.Errorf("expected domain without slash but got %s", domain)
}
certPath := s.certLocation + "/" + domain + ".pem"
Copy link

Choose a reason for hiding this comment

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

You should use filepath here

Copy link
Author

Choose a reason for hiding this comment

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

What from filepath would be useful?

Copy link

Choose a reason for hiding this comment

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

internal/common/spiretrustsource.go Outdated Show resolved Hide resolved
internal/common/filetrustsource.go Outdated Show resolved Hide resolved
Peyton Walters added 7 commits January 15, 2020 11:45
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Peyton Walters added 2 commits January 15, 2020 17:18
Signed-off-by: Peyton Walters <[email protected]>
Signed-off-by: Peyton Walters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Unit Tests
3 participants