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

Support --skip-delete to skip deletion of resources created during tests #484

Merged

Conversation

jbarrick-mesosphere
Copy link
Member

What type of PR is this?

/kind feature

What this PR does / why we need it:

Support --skip-delete to skip deletion of resources created during tests.

This helps with debugging tests.

Also refactors the test harness CLI parsing to load configuration:

  1. If --config is set, from the config specified.
  2. If --config is not set and kudo-test.yaml exists, it loads it from kudo-test.yaml.
  3. Arguments set on the command line override configuration from the configuration.

Previously you could only use CLI arguments or a configuration.

Which issue(s) this PR fixes:

Fixes #377

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[test harness] Support --skip-delete to skip deletion of resources created during tests

…sts.

This helps with debugging tests.

Fixes kudobuilder#377.

Also refactors the test harness CLI parsing to load configuration:

1. If `--config` is set, from the config specified.
2. If `--config` is not set and `kudo-test.yaml` exists, it loads it from `kudo-test.yaml`.
3. Arguments set on the command line override configuration from the configuration.

Previously you could only use CLI arguments or a configuration.
@@ -86,6 +87,30 @@ For more detailed documentation, visit: https://kudo.dev/docs/testing`,
}
}

if isSet(flags, "crd-dir") {
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 what is the benefit of this vs having options.CRDDir directly in testCmd.Flags().StringVar? If not set, you'll have the default value for that type anyway, right? I guess it at least deserves a comment since it's not obvious (at least to me)

Copy link
Member Author

Choose a reason for hiding this comment

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

For consistency, mainly. It's more important below with the booleans - I want to know the difference between not set, false, and true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added some comments

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

Got it, thanks

@jbarrick-mesosphere jbarrick-mesosphere merged commit 9175502 into kudobuilder:master Jul 2, 2019
@jbarrick-mesosphere jbarrick-mesosphere deleted the test-skip-delete branch July 2, 2019 17:26
@jbarrick-mesosphere jbarrick-mesosphere added this to the v0.4.0 milestone Jul 17, 2019
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.

KUDO test harness should support optionally not deleting test resources
2 participants