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 tests scaffolded in default layout #3670

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

Lauquik
Copy link
Contributor

@Lauquik Lauquik commented Oct 24, 2023

Description

adding testing scaffold for the default layout of our project

Test perfoms the follwoing tasks

  • Building the Operator Image:
  • Loading the Operator Image to Kind:
  • Deploying the Controller-Manager:
  • Validating Controller-Manager Pod:

Fixes

#3605

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 24, 2023
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 24, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @Lauquik. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 24, 2023
test/e2e/ci.sh Outdated Show resolved Hide resolved
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Great work, but I think we still require some adjusts

  • Rebase with master
  • Do not change the current e2e tests to try run the sample tests. The e2e tests executed run the e2e tests scaffolds for kubebuilder itself. We need a new GitHub action that will run the sample tests and be configured with kind
  • Ensure that the final scaffold has only what is required for end users.

@camilamacedo86
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 24, 2023
@Lauquik Lauquik force-pushed the laukik branch 3 times, most recently from bf5b3d1 to 1359d10 Compare October 25, 2023 17:05
test/e2e/ci.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@Lauquik
Copy link
Contributor Author

Lauquik commented Oct 26, 2023

@camilamacedo86 i have created a whole new workflow for sample project-v4 and removed the sample-e2e.sh script, is it okay? or require any changes, i'm open to receiving any suggestion and guidance

@camilamacedo86
Copy link
Member

Hi @Lauquik,

@camilamacedo86 i have created a whole new workflow for sample project-v4 and removed the sample-e2e.sh script, is it okay? or require any changes, i'm open to receiving any suggestion and guidance

  • The samples should continue to be generated as they currently are.
  • The existing checks should remain operational in their current form.

That said, to ensure the integrity of our new e2e tests, we need a dedicated GitHub Action. While it's not necessary to test every single sample (and if doing so simplifies the process, that's acceptable), a streamlined approach might be:

  1. Set up a new GitHub Action.
  2. Have this action install kind.
  3. Run the e2e tests that have been scaffolded for example for project-v4.

Makefile Outdated Show resolved Hide resolved
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Hi @Lauquik

Very terrific great work 🥇
I think we have just a couple of nits to sort out, and then it is good to fly.

@Lauquik
Copy link
Contributor Author

Lauquik commented Oct 26, 2023

@camilamacedo86 i think you are right removing the test dir is not the best approach, i will think of some other solution

@Lauquik
Copy link
Contributor Author

Lauquik commented Oct 26, 2023

@camilamacedo86 I have modified the scaffolded Makefile to exclude the e2e test when running 'make test,' so there's no need to remove the test directories explicitly

Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Great work 🥇
It is great for me so I am approving this one

/approved

Let's give some time to see if others also want to do some review.
If nobody came with a second review or objections we can lgtm this one.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2023
Copy link
Contributor

@Kavinjsir Kavinjsir left a comment

Choose a reason for hiding this comment

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

Great job @Lauquik 👍🏼 The PR Looks good to me!

Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

It seems good to fly
we can always push improvements and etc in a follow up

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, Kavinjsir, Lauquik

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit cc9227e into kubernetes-sigs:master Nov 3, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants