Skip to content

helm:replace test in shell for go#3423

Merged
camilamacedo86 merged 2 commits intooperator-framework:masterfrom
camilamacedo86:helm-plugin-go-test
Jul 22, 2020
Merged

helm:replace test in shell for go#3423
camilamacedo86 merged 2 commits intooperator-framework:masterfrom
camilamacedo86:helm-plugin-go-test

Conversation

@camilamacedo86
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 commented Jul 14, 2020

Description of the change:

  • Replace the tests in the shell for Go
  • Centralize ReplaceInFile in the utils

Motivation of the change:
Replace the tests made to check the helm implementation in a shell script for golang

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 14, 2020
@camilamacedo86 camilamacedo86 changed the title WIP; feat:replace test in shell for go WIP: feat:replace test in shell for go Jul 14, 2020
@camilamacedo86 camilamacedo86 requested a review from asmacdo July 14, 2020 21:22
@camilamacedo86 camilamacedo86 changed the title WIP: feat:replace test in shell for go WIP: helm:replace test in shell for go Jul 14, 2020
@camilamacedo86 camilamacedo86 changed the title WIP: helm:replace test in shell for go helm:replace test in shell for go Jul 15, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2020
@camilamacedo86 camilamacedo86 added language/helm Issue is related to a Helm operator project kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form labels Jul 15, 2020
Comment thread hack/tests/e2e-helm.sh Outdated
Comment thread hack/tests/e2e-helm.sh Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_suite.go Outdated
Comment thread test/e2e-helm/e2e_test.go
@camilamacedo86 camilamacedo86 changed the title helm:replace test in shell for go WIP: helm:replace test in shell for go Jul 17, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2020
@camilamacedo86 camilamacedo86 changed the title WIP: helm:replace test in shell for go helm:replace test in shell for go Jul 17, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2020
@camilamacedo86 camilamacedo86 changed the title helm:replace test in shell for go WIP : helm:replace test in shell for go Jul 18, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 18, 2020
@camilamacedo86 camilamacedo86 changed the title WIP : helm:replace test in shell for go helm:replace test in shell for go Jul 18, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 18, 2020
Comment thread hack/lib/common.sh Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_suite_test.go
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
@camilamacedo86 camilamacedo86 requested a review from estroz July 20, 2020 23:11
Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

IMO, we shouldn't deploy/undeploy the project multiple times for different tests, since that will cause the test to take longer with little benefit.

For the tests that require a running operator, I think there should be one test file where the root level BeforeEach/AfterEach do the make deploy/make undeploy steps. Then all the tests that need a running operator (CR reconciliation, metrics) happen in that context.

Comment thread hack/tests/e2e-helm.sh Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_metrics_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_olm_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_locally_test.go
Comment thread test/e2e-helm/e2e_helm_project_locally_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_locally_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_locally_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_on_cluster_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_project_locally_test.go
Comment thread test/e2e-helm/e2e_helm_suite_test.go Outdated
Comment thread test/e2e-helm/e2e_helm_suite_test.go Outdated
Comment thread hack/tests/e2e-helm.sh Outdated
Comment thread test/e2e-helm/e2e_helm_cluster_test.go Outdated
Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020
@openshift-ci-robot
Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@camilamacedo86
Copy link
Copy Markdown
Contributor Author

Hi @estroz,

All your suggestions were addressed as well. I am moving forward here for we are able to do the next task that requires these changes (e.g OLM sub running for noGo ones and its tests). However, please feel free to check again and let us know if has some detail/change that you think that should be applied. We can do it in a follow-up.

@camilamacedo86 camilamacedo86 merged commit 9115ab1 into operator-framework:master Jul 22, 2020
@camilamacedo86 camilamacedo86 deleted the helm-plugin-go-test branch July 22, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form language/helm Issue is related to a Helm operator project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants