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

Add e2e scaffolding using Testify Suites #3354

Closed

Conversation

codegold79
Copy link
Contributor

Per @nrb's review of PR #3133, this PR splits out the e2e Testify Suites framework and supporting code.

Signed-off-by: F. Gold fgold@vmware.com

codegold79 added a commit to codegold79/velero that referenced this pull request Jan 26, 2021
@zubron zubron added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jan 26, 2021
Signed-off-by: F. Gold <fgold@vmware.com>
Copy link
Contributor

@zubron zubron left a comment

Choose a reason for hiding this comment

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

Thanks for splitting this out into a separate PR, @codegold79!

I think it will be nice to have the option on how to write tests (using testify vs ginkgo) but I'm concerned about the long term maintainability of having two separate build/run processes. I'm not familiar with how ginkgo tests run, but is it possible to have both regular go tests and ginkgo tests in one package? If yes, then I think that ultimately we should move any tests using that approach inside the e2e package rather than creating a new one. Otherwise, it will be very easy for these to get out of sync and increase the maintenance burden. Also, I think that some of the functionality added in this package will be useful for the main e2e package :)

I don't want to block your progress on this though, @codegold79, so I think we can address the overall structure of the e2e packages in a follow up and I don't want to consider it a blocker for this PR. I mostly just want to make sure that we've considered the impact on the rest of the e2e tests moving forward as there is a lot of work happening in that area.

@dsu-igeek What are your thoughts on this?

)

var (
bslBucket = flag.String("bucket", "", "name of the object storage bucket where backups from e2e tests should be stored. Required.")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm apprehensive about repeating these flag definitions again. I don't want us to have two separate sets of flags that are passed through to a shared set of utility functions. I know that #3286 is changing some of these flags in the e2e and adding more that will need to handled. I don't want to block your progress on this and #3133 though, as you've already had to wait a long time on that :)

I think there are probably general improvements to the flags in the main e2e package (such as using a struct to contain them) that can be made which would make sharing across these two packages easier. I'll open a follow up issue for this.

return nil
}

func providerPlugins(provider string) []string {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could export getProviderPlugins so that you can use that function.

func InstallCRD(ctx context.Context, crdFile, ns string) error {
fmt.Printf("Install CRD %s.\n", crdFile)

cmd := exec.CommandContext(ctx, "kubectl", "apply", "-f", crdFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be addressed in #3133, but I think it would be better to specify the namespace where the CRDs are being installed using the ns value passed in rather than having to know the namespace specified in the YAML.

Makefile Show resolved Hide resolved
Signed-off-by: F. Gold <fgold@vmware.com>
@github-actions github-actions bot requested a review from zubron January 28, 2021 00:58
@codegold79
Copy link
Contributor Author

codegold79 commented Jan 28, 2021

Thank you, @zubron, for your review. I completely agree that it is difficult to maintain a parallel set of code (i.e. duplicated) and already some of my code is outdated.

I had wanted to provide an example of using Testify Suites for e2e tests so people would have something to compare with Ginkgo in Velero. However, per @nrb in a code review meeting for PR #3133, given the limited resources (including time), we won't be rewriting the Ginkgo tests. So I'll write tests for PR #3133 to work with the existing Ginkgo framework. I have copied over the Testify Suites code to this PR for future reference and will close the PR.

@codegold79 codegold79 closed this Jan 28, 2021
nrb pushed a commit that referenced this pull request Feb 16, 2021
* Restore API group version by priority

Signed-off-by: F. Gold <fgold@vmware.com>

* Add changelog

Signed-off-by: F. Gold <fgold@vmware.com>

* Correct spelling

Signed-off-by: F. Gold <fgold@vmware.com>

* Refactor userResourceGroupVersionPriorities(...) to accept config map, adjust unit test

Signed-off-by: F. Gold <fgold@vmware.com>

* Move some unit tests into e2e

Signed-off-by: F. Gold <fgold@vmware.com>

* Add three e2e tests using Testify Suites

Summary of changes

Makefile - add testify e2e test target
go.sum - changed with go mod tidy
pkg/install/install.go - increased polling timeout
test/e2e/restore_priority_group_test.go - deleted
test/e2e/restore_test.go - deleted
test/e2e/velero_utils.go - made restic optional in velero install
test/e2e_testify/Makefile - makefile for testify e2e tests
test/e2e_testify/README.md - example command for running tests
test/e2e_testify/common_test.go - helper functions
test/e2e_testify/e2e_suite_test.go - prepare for tests and run
test/e2e_testify/restore_priority_apigv_test.go - test cases

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review

Signed-off-by: F. Gold <fgold@vmware.com>

* Wait for pods in e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Remove testify suites e2e scaffolding moved to PR #3354

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @brito-rafa and Velero maintainers code reviews

- Made changes suggested by @brito-rafa in GitHub.
- We had a code review meeting with @carlisia, @dsu-igeek, @zubron, and @nrb
- and changes were made based on their suggetions:
  - pull in logic from 'meetsAPIGVResotreReqs()' to restore.go.
  - add TODO to remove APIGroupVersionFeatureFlag check
  - have feature flag and backup version format checks in separate `if` statements.
  - rename variables to be sourceGVs, targetGVs, and userGVs.

Signed-off-by: F. Gold <fgold@vmware.com>

* Convert Testify Suites e2e tests to existing Ginkgo framework

Signed-off-by: F. Gold <fgold@vmware.com>

* Made changes per @zubron PR review

Signed-off-by: F. Gold <fgold@vmware.com>

* Run go mod tidy after resolving go.sum merge conflict

Signed-off-by: F. Gold <fgold@vmware.com>

* Add feature documentation to velero.io site

Signed-off-by: F. Gold <fgold@vmware.com>

* Add config map e2e test; rename e2e test file and name

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Move CRDs and CRs to testdata folder

Signed-off-by: F. Gold <fgold@vmware.com>

* Fix typos in cert-manager to pass codespell CICD check

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review round 2

- make checkAndReadDir function private
- add info level messages when priorties 1-3 API group versions can not be used

Signed-off-by: F. Gold <fgold@vmware.com>

* Make user config map rules less strict

Signed-off-by: F. Gold <fgold@vmware.com>

* Update e2e test image version in example

Signed-off-by: F. Gold <fgold@vmware.com>

* Update case A music-system controller code

Signed-off-by: F. Gold <fgold@vmware.com>

* Documentation updates

Signed-off-by: F. Gold <fgold@vmware.com>

* Update migration case documentation

Signed-off-by: F. Gold <fgold@vmware.com>
dharmab pushed a commit to dharmab/velero that referenced this pull request May 25, 2021
* Restore API group version by priority

Signed-off-by: F. Gold <fgold@vmware.com>

* Add changelog

Signed-off-by: F. Gold <fgold@vmware.com>

* Correct spelling

Signed-off-by: F. Gold <fgold@vmware.com>

* Refactor userResourceGroupVersionPriorities(...) to accept config map, adjust unit test

Signed-off-by: F. Gold <fgold@vmware.com>

* Move some unit tests into e2e

Signed-off-by: F. Gold <fgold@vmware.com>

* Add three e2e tests using Testify Suites

Summary of changes

Makefile - add testify e2e test target
go.sum - changed with go mod tidy
pkg/install/install.go - increased polling timeout
test/e2e/restore_priority_group_test.go - deleted
test/e2e/restore_test.go - deleted
test/e2e/velero_utils.go - made restic optional in velero install
test/e2e_testify/Makefile - makefile for testify e2e tests
test/e2e_testify/README.md - example command for running tests
test/e2e_testify/common_test.go - helper functions
test/e2e_testify/e2e_suite_test.go - prepare for tests and run
test/e2e_testify/restore_priority_apigv_test.go - test cases

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review

Signed-off-by: F. Gold <fgold@vmware.com>

* Wait for pods in e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Remove testify suites e2e scaffolding moved to PR vmware-tanzu#3354

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @brito-rafa and Velero maintainers code reviews

- Made changes suggested by @brito-rafa in GitHub.
- We had a code review meeting with @carlisia, @dsu-igeek, @zubron, and @nrb
- and changes were made based on their suggetions:
  - pull in logic from 'meetsAPIGVResotreReqs()' to restore.go.
  - add TODO to remove APIGroupVersionFeatureFlag check
  - have feature flag and backup version format checks in separate `if` statements.
  - rename variables to be sourceGVs, targetGVs, and userGVs.

Signed-off-by: F. Gold <fgold@vmware.com>

* Convert Testify Suites e2e tests to existing Ginkgo framework

Signed-off-by: F. Gold <fgold@vmware.com>

* Made changes per @zubron PR review

Signed-off-by: F. Gold <fgold@vmware.com>

* Run go mod tidy after resolving go.sum merge conflict

Signed-off-by: F. Gold <fgold@vmware.com>

* Add feature documentation to velero.io site

Signed-off-by: F. Gold <fgold@vmware.com>

* Add config map e2e test; rename e2e test file and name

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Move CRDs and CRs to testdata folder

Signed-off-by: F. Gold <fgold@vmware.com>

* Fix typos in cert-manager to pass codespell CICD check

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review round 2

- make checkAndReadDir function private
- add info level messages when priorties 1-3 API group versions can not be used

Signed-off-by: F. Gold <fgold@vmware.com>

* Make user config map rules less strict

Signed-off-by: F. Gold <fgold@vmware.com>

* Update e2e test image version in example

Signed-off-by: F. Gold <fgold@vmware.com>

* Update case A music-system controller code

Signed-off-by: F. Gold <fgold@vmware.com>

* Documentation updates

Signed-off-by: F. Gold <fgold@vmware.com>

* Update migration case documentation

Signed-off-by: F. Gold <fgold@vmware.com>
ywk253100 pushed a commit to ywk253100/velero that referenced this pull request Jun 29, 2021
* Restore API group version by priority

Signed-off-by: F. Gold <fgold@vmware.com>

* Add changelog

Signed-off-by: F. Gold <fgold@vmware.com>

* Correct spelling

Signed-off-by: F. Gold <fgold@vmware.com>

* Refactor userResourceGroupVersionPriorities(...) to accept config map, adjust unit test

Signed-off-by: F. Gold <fgold@vmware.com>

* Move some unit tests into e2e

Signed-off-by: F. Gold <fgold@vmware.com>

* Add three e2e tests using Testify Suites

Summary of changes

Makefile - add testify e2e test target
go.sum - changed with go mod tidy
pkg/install/install.go - increased polling timeout
test/e2e/restore_priority_group_test.go - deleted
test/e2e/restore_test.go - deleted
test/e2e/velero_utils.go - made restic optional in velero install
test/e2e_testify/Makefile - makefile for testify e2e tests
test/e2e_testify/README.md - example command for running tests
test/e2e_testify/common_test.go - helper functions
test/e2e_testify/e2e_suite_test.go - prepare for tests and run
test/e2e_testify/restore_priority_apigv_test.go - test cases

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review

Signed-off-by: F. Gold <fgold@vmware.com>

* Wait for pods in e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Remove testify suites e2e scaffolding moved to PR vmware-tanzu#3354

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @brito-rafa and Velero maintainers code reviews

- Made changes suggested by @brito-rafa in GitHub.
- We had a code review meeting with @carlisia, @dsu-igeek, @zubron, and @nrb
- and changes were made based on their suggetions:
  - pull in logic from 'meetsAPIGVResotreReqs()' to restore.go.
  - add TODO to remove APIGroupVersionFeatureFlag check
  - have feature flag and backup version format checks in separate `if` statements.
  - rename variables to be sourceGVs, targetGVs, and userGVs.

Signed-off-by: F. Gold <fgold@vmware.com>

* Convert Testify Suites e2e tests to existing Ginkgo framework

Signed-off-by: F. Gold <fgold@vmware.com>

* Made changes per @zubron PR review

Signed-off-by: F. Gold <fgold@vmware.com>

* Run go mod tidy after resolving go.sum merge conflict

Signed-off-by: F. Gold <fgold@vmware.com>

* Add feature documentation to velero.io site

Signed-off-by: F. Gold <fgold@vmware.com>

* Add config map e2e test; rename e2e test file and name

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Move CRDs and CRs to testdata folder

Signed-off-by: F. Gold <fgold@vmware.com>

* Fix typos in cert-manager to pass codespell CICD check

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review round 2

- make checkAndReadDir function private
- add info level messages when priorties 1-3 API group versions can not be used

Signed-off-by: F. Gold <fgold@vmware.com>

* Make user config map rules less strict

Signed-off-by: F. Gold <fgold@vmware.com>

* Update e2e test image version in example

Signed-off-by: F. Gold <fgold@vmware.com>

* Update case A music-system controller code

Signed-off-by: F. Gold <fgold@vmware.com>

* Documentation updates

Signed-off-by: F. Gold <fgold@vmware.com>

* Update migration case documentation

Signed-off-by: F. Gold <fgold@vmware.com>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
* Restore API group version by priority

Signed-off-by: F. Gold <fgold@vmware.com>

* Add changelog

Signed-off-by: F. Gold <fgold@vmware.com>

* Correct spelling

Signed-off-by: F. Gold <fgold@vmware.com>

* Refactor userResourceGroupVersionPriorities(...) to accept config map, adjust unit test

Signed-off-by: F. Gold <fgold@vmware.com>

* Move some unit tests into e2e

Signed-off-by: F. Gold <fgold@vmware.com>

* Add three e2e tests using Testify Suites

Summary of changes

Makefile - add testify e2e test target
go.sum - changed with go mod tidy
pkg/install/install.go - increased polling timeout
test/e2e/restore_priority_group_test.go - deleted
test/e2e/restore_test.go - deleted
test/e2e/velero_utils.go - made restic optional in velero install
test/e2e_testify/Makefile - makefile for testify e2e tests
test/e2e_testify/README.md - example command for running tests
test/e2e_testify/common_test.go - helper functions
test/e2e_testify/e2e_suite_test.go - prepare for tests and run
test/e2e_testify/restore_priority_apigv_test.go - test cases

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review

Signed-off-by: F. Gold <fgold@vmware.com>

* Wait for pods in e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Remove testify suites e2e scaffolding moved to PR vmware-tanzu#3354

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @brito-rafa and Velero maintainers code reviews

- Made changes suggested by @brito-rafa in GitHub.
- We had a code review meeting with @carlisia, @dsu-igeek, @zubron, and @nrb
- and changes were made based on their suggetions:
  - pull in logic from 'meetsAPIGVResotreReqs()' to restore.go.
  - add TODO to remove APIGroupVersionFeatureFlag check
  - have feature flag and backup version format checks in separate `if` statements.
  - rename variables to be sourceGVs, targetGVs, and userGVs.

Signed-off-by: F. Gold <fgold@vmware.com>

* Convert Testify Suites e2e tests to existing Ginkgo framework

Signed-off-by: F. Gold <fgold@vmware.com>

* Made changes per @zubron PR review

Signed-off-by: F. Gold <fgold@vmware.com>

* Run go mod tidy after resolving go.sum merge conflict

Signed-off-by: F. Gold <fgold@vmware.com>

* Add feature documentation to velero.io site

Signed-off-by: F. Gold <fgold@vmware.com>

* Add config map e2e test; rename e2e test file and name

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Move CRDs and CRs to testdata folder

Signed-off-by: F. Gold <fgold@vmware.com>

* Fix typos in cert-manager to pass codespell CICD check

Signed-off-by: F. Gold <fgold@vmware.com>

* Make changes per @nrb code review round 2

- make checkAndReadDir function private
- add info level messages when priorties 1-3 API group versions can not be used

Signed-off-by: F. Gold <fgold@vmware.com>

* Make user config map rules less strict

Signed-off-by: F. Gold <fgold@vmware.com>

* Update e2e test image version in example

Signed-off-by: F. Gold <fgold@vmware.com>

* Update case A music-system controller code

Signed-off-by: F. Gold <fgold@vmware.com>

* Documentation updates

Signed-off-by: F. Gold <fgold@vmware.com>

* Update migration case documentation

Signed-off-by: F. Gold <fgold@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants