-
Notifications
You must be signed in to change notification settings - Fork 592
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
e2e: nodeselector in apiserversauce #7627
Conversation
Hi @sadath-12. Thanks for your PR. I'm waiting for a knative 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. |
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.
Hi @sadath-12, we are moving away from the older style e2e tests to the new rekt tests in eventing. Would you be able to re-write this in the rekt folder as outlined in the issue? For more info on how rekt tests work, take a look at how the tests were added in this commit: 74e165a#diff-942d2d7cd8e70afcaacfa78fd686671629c18c3708a35721aa08f59f9876692f, as well as look at the reconciler-test repo.
thanks @Cali0707 . |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7627 +/- ##
==========================================
- Coverage 74.44% 74.01% -0.44%
==========================================
Files 262 262
Lines 15077 15172 +95
==========================================
+ Hits 11224 11229 +5
- Misses 3247 3337 +90
Partials 606 606 ☔ View full report in Codecov by Sentry. |
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
thank you @Cali0707 . The CI are quite flaky and it would be difficult to find our tests so I'm providing the output I got from my machine to confirm it works
|
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.
Thanks for your work on this so far @sadath-12
One thing to note about reconciler-tests is that the tests run multiple step functions concurrently in each phase. So everything in f.Setup()
runs concurrently, and they will all finish before anything in f.Requirement()
runs.
So, to make sure something happens before something else, you need to put them into different phases
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
@Cali0707 any updates ? |
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
/ok-to-test |
/cc |
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
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.
Thanks for all the work on this @sadath-12
Just a few nits and then I think it will be good to merge
Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>
Thank you @Cali0707 |
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.
/lgtm
/approve
Thanks for your work on this @sadath-12 !
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, sadath-12 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #7615
Proposed Changes
Added e2e test to make sure
nodeselector
is picked inapiserversauce
deployment correctly fromfeatureFlags
Pre-review Checklist
Release Note
Docs