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

upgrade: unit tests #2672

Merged
merged 2 commits into from
Apr 10, 2019
Merged

upgrade: unit tests #2672

merged 2 commits into from
Apr 10, 2019

Conversation

siggy
Copy link
Member

@siggy siggy commented Apr 9, 2019

This change introduces some unit tests on individual methods in the
upgrade code path, along with some minor cleanup.

Part of #2637

Signed-off-by: Andrew Seigner [email protected]

This change introduces some unit tests on individual methods in the
upgrade code path, along with some minor cleanup.

Part of #2637

Signed-off-by: Andrew Seigner <[email protected]>
@siggy siggy self-assigned this Apr 9, 2019
@admc admc mentioned this pull request Apr 9, 2019
25 tasks
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 9, 2019

Integration test results for 9ba396d: success 🎉
Log output: https://gist.github.com/92a9d5600271a5445a520c69fd12601a

Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

LGTM. Just some minor comments and TIOLI.

@@ -482,6 +483,7 @@ func toPromLogLevel(level string) string {
}
}

// TODO: are `installValues.Configs` and `configs` redundant?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think installValues.Configs is used in the chart/templates/config.yaml helm chart template. Essentially, it's the JSON of configs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think my question is more around: If installValues.Configs and configs are just different representations of the same data, can we just keep one copy of that data around, and generate one version from the other on-demand? Any time we have multiple copies of the same data, I get concerned about which version is the source of truth, and what happens if they are not in sync.

cli/cmd/upgrade_test.go Show resolved Hide resolved
cli/cmd/upgrade_test.go Outdated Show resolved Hide resolved
cli/cmd/upgrade_test.go Outdated Show resolved Hide resolved
cli/cmd/upgrade_test.go Outdated Show resolved Hide resolved
@siggy siggy force-pushed the siggy/update-upgrade-unit branch from 9003ace to 9ba396d Compare April 10, 2019 20:36
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 9ba396d: success 🎉
Log output: https://gist.github.com/9bb41a628a74c8d2f7abcb81ededc1d4

@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 3ca1c3c: success 🎉
Log output: https://gist.github.com/7990675f99412dbd0cc35e8af9f81b6a

@siggy siggy merged commit 43cb3f8 into master Apr 10, 2019
@siggy siggy deleted the siggy/update-upgrade-unit branch April 10, 2019 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants