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

Enhance e2e Test by Adding a Method to Generate Project without Webhooks #3618

Closed
camilamacedo86 opened this issue Sep 13, 2023 · 16 comments · Fixed by #3647
Closed

Enhance e2e Test by Adding a Method to Generate Project without Webhooks #3618

camilamacedo86 opened this issue Sep 13, 2023 · 16 comments · Fixed by #3647
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@camilamacedo86
Copy link
Member

camilamacedo86 commented Sep 13, 2023

What do you want to happen?

Description:

In the current e2e tests, we enable both cert-manager and webhooks to verify if everything functions as expected. The respective code can be found here: generate_test.go#L37-L210.

Task Details:

  • Introduce a new method to generate the project without activating the webhooks.
  • Use this new method in our e2e tests and ensure the functionality is tested in a similar manner to the existing tests.
  • Make sure to call the Run method as demonstrated in the current e2e test code: generate_test.go#L37-L210.

Additional Information:

This enhancement ensures that we are better testing the possible configurations, both with and without webhooks. It will ensure that changes like: #3585 does not broke the default scaffold.

Extra Labels

No response

@camilamacedo86 camilamacedo86 added kind/feature Categorizes issue or PR as related to a new feature. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Sep 13, 2023
@Sajiyah-Salat
Copy link
Contributor

Hello @camilamacedo86 I pretty much interested in this issue because its needed and two pr are on hold because of this. I want to work on this issue but I have no experience working with tests. Can you please help me like how to start? What should I refer to ? It will be a great help.

@camilamacedo86

This comment was marked as resolved.

@Sajiyah-Salat

This comment was marked as resolved.

@Sajiyah-Salat
Copy link
Contributor

Sajiyah-Salat commented Sep 15, 2023

@camilamacedo86 I understood that the we need to add a new block in v4 func to validate the test without gnerating a webhook. I am confused in which tag/label is to use. Also I have never wrote tests so I wonder how it will be written with a logic.

@Sajiyah-Salat
Copy link
Contributor

I created a cluster named kubebuilder.
Changed directory to test/e2e and run make test-e2e
there I got an error make: *** No rule to make target 'test-e2e'. Stop.
Is there anything I am missing here.

@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Sep 16, 2023

HI @Sajiyah-Salat,

I created a cluster named kubebuilder.
Changed directory to test/e2e and run make test-e2e
there I got an error make: *** No rule to make target 'test-e2e'. Stop.
Is there anything I am missing here.

To know what targets the project has and how to use them you can:

@camilamacedo86 I understood that the we need to add a new block in v4 func to validate the test without gnerating a webhook. I am confused in which tag/label is to use. Also I have never wrote tests so I wonder how it will be written with a logic.

@typeid
Copy link
Contributor

typeid commented Sep 17, 2023

@camilamacedo86 @Sajiyah-Salat am I free to assign myself on this?

@Sajiyah-Salat
Copy link
Contributor

Hello @typeid I am trying to work on this. But we can work together as well.

@Sajiyah-Salat
Copy link
Contributor


From what I have understand we need to add new method here just like this but without this block. Please let me know if I have understood it fully or now. Also do we just need to add go codeblock or also to write script or it will cover directly in end to end script.

@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Sep 23, 2023

Hi @Sajiyah-Salat,

At the moment, we have a singular test where we scaffold APIs + webhooks and enable all options.

Our goal is to create another mock, akin to GenerateV4 (feel free to copy and paste), which will include everything except the webhooks.

Here are the steps broken down:

  1. Copy and Paste Method: Copy and paste the method GenerateV4.

    • You will end up with GenerateV4 and a new method, GenerateV4WithoutWebhooks.
  2. Modify the New Method: In GenerateV4WithoutWebhooks, avoid calling kubebuilder create webhook as you identified.

    • Additionally, do not uncomment the kustomize files related to enabling webhooks. See: generate_test.go
  3. Run and Test Both Methods: Lastly, ensure you run and test both the old and the new methods. Reference: plugin_cluster_test.go

    • Add something like:
   It("should generate a runnable project without webhooks"+
   " with restricted pods", func() {
      kbc.IsRestricted = true
      GenerateV4WithoutWebhooks(kbc) 
      Run(kbc, false)  // here you are passsing false to for a bool like hasWebhooks so that you can skip the webhooks checked based on it
   })

For your new mock test GenerateV4WithoutWebhooks , the following checks should not be executed:

So that means you need to do something like:

func Run(kbc *utils.TestContext, hasWebhooks bool) {

     if hasWebhooks {
         // Check 1 code
     }

     if hasWebhooks {
         // Check 2 code
     }
}

I hope that helps you out.

@Sajiyah-Salat
Copy link
Contributor

Thanks for the detailed answer I will execute it soon and create a tested pr as well.
/assign

@Sajiyah-Salat
Copy link
Contributor

on the second point you are talking about not uncommenting. From that you mean I should not uncomment the yaml file or not to write the uncomment code that you have selected in point 2.

@Sajiyah-Salat
Copy link
Contributor

in second check we are validating webhook which is not the goal. do we need to pass that check. really?

@Sajiyah-Salat
Copy link
Contributor

Also after passing the check the work is done I dont need to write the script or if else condition that it should pass two checks. You have given that just for understanding purpose. or should I write.

@Sajiyah-Salat
Copy link
Contributor

Sorry for asking so many doubts. I just want to be clear before opening a pr.

@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Oct 1, 2023

@Sajiyah-Salat,

Addressing this issue was more intricate than initially anticipated. The primary approach involved adding tests that, as expected, highlighted the failure points. The plan was to then introduce a fix that would cause these tests to pass, indicating the issue was resolved.

However, upon a closer examination, it became evident that the preliminary solution wouldn't adequately address the root of the problem. As a result, I've had to revisit our strategy.

I've made revisions to both the corrective approach and the tests to ensure robustness. For details on these changes and the final solution, please refer to the updated PR linked below:

#3647

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
3 participants