-
Notifications
You must be signed in to change notification settings - Fork 69
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
upgraded ginkgo on tests to ginkgo 2.0 #560
Conversation
Hi @mperetzred. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
3d31160
to
2b922cd
Compare
2b922cd
to
f47d735
Compare
Please make sure this passes 4.7/4.8 e2e more than just once before merge :) PRETTY PLEASE :) |
log.Printf("Creating VolumeSnapshot for CSI backuprestore of %s", brCase.Name) | ||
err = installApplication(dpaCR.Client, "./sample-applications/gp2-csi/volumeSnapshotClass.yaml") | ||
Expect(err).ToNot(HaveOccurred()) | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mperetzred could you please explain, reason for removing this else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for it, using aws label instead, and it will be filtered if cluster profile is not aws (check the filters on the makefile)
@@ -170,7 +165,7 @@ var _ = Describe("AWS backup restore tests", func() { | |||
} | |||
|
|||
}, | |||
Entry("MSSQL application CSI", BackupRestoreCase{ | |||
Entry("MSSQL application CSI", Label("aws"), BackupRestoreCase{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this label has been applied only to first Entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of having a condition inside the test logic:
if brCase.BackupRestoreType == csi {
if clusterProfile == "aws" {
log.Printf("Creating VolumeSnapshot for CSI backuprestore of %s", brCase.Name)
err = installApplication(dpaCR.Client, "./sample-applications/gp2-csi/volumeSnapshotClass.yaml")
Expect(err).ToNot(HaveOccurred())
} else {
@weshayutin, it seems like it fails due to the same problem you have on CI. The mssql deployment doesn't get ready. |
/retest-required |
sorry, accidentally closed it. |
/retest |
@weshayutin, all checks have passed, want me to do a /retest again? |
@mperetzred the instability of the jobs is not your fault, nor am I saying it's this particular patch. With that said, we're working to improve that. The e2e on this pr 4.8 atm has a 90% pass rate in periodic We need to see this pass more than once on 4.8 please and thank you!!! |
/retest |
Nice.. +1 /recheck ( why not :) ) |
hrm.. new tests did not yet trigger afaict.. |
/test pull-ci-openshift-oadp-operator-master-4.7-operator-e2e |
@weshayutin: The specified target(s) for
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test 4.7-operator-e2e |
/test 4.8-operator-e2e |
/pony Twilight Sparkle |
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest |
/test 4.8-operator-e2e |
1 similar comment
/test 4.8-operator-e2e |
last 4.8 was flake :) |
@mperetzred: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
thanks for your patience @mperetzred :) +1 to merge from me.. after reviewing |
…n updateRegistrySecret (#679) * Replace carriage return when getting provider secrets for registry + E2E changes (#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * Multi cloud E2E Updates (#568) * Adding changes from ci-multi-cloud branch * Updating flags in makefile * Updating flags in makefile * Updating flags in makefile * Update defaults * Fix helpers * Cleaning up makefile * Update makefile * Update kustomization * Cleaning up Makefile * Adding changes for AWS CredentialFile * Handling no default backuplocations * Removing comments * Adding config check in helpers * Fix AWS Test case * Fix basic review comments * genericTests... * Changing Credentials Mount Name for BSL when CredentialsFile key is used in Config - Fixes CI (#637) * Changing Credentials Mount Name for BSL for GCP * Fix DefaultPlugin for credentialsFile * E2E Tests: Fix azure templating name. (#573) * Fixing Azure to run locally * Aligning json * Handling Azure Parameters in CI Env using templates. (#582) * Adding azure parameters * Adding Openshift CI params * Update Test Suite and Helpers * Adding cred ref * Adding cred ref * Changing azure oadp cred dir to tmp * Fixing duplicate error * Removing OpenShift CI * Changing test instance name * Changing the AWS BSL Profile to default * Changing BslMountPath variable in registry controller * Replace carriage return when getting provider secrets for registry + E2E changes (#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * fix missing err return for updateRegistrySecret function * go fmt * upgraded ginkgo on tests to ginkgo 2.0 (#560) Co-authored-by: Nitish Srivastava <43022982+nitishSr@users.noreply.github.com> Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> Co-authored-by: Deepak Raj D S <drajds@redhat.com> Co-authored-by: Shubham Pampattiwar <shubhampampattiwar7@gmail.com> Co-authored-by: Maya Peretz <mperetz@redhat.com>
Upgraded ginkgo to version 2.0, so it will be possible to use labels in order to filter tests in CI, instead of using conditions within the test logic.
Left ginkgo 1.16 because it's used in controllers/suite_test.go, and it uses timeout as part of BeforeSuite function, which is probably deprecated in ginkgo 2..