ingress-nginx icon indicating copy to clipboard operation
ingress-nginx copied to clipboard

Adding a simpler interface for the HTTP request library

Open knabben opened this issue 3 years ago • 6 comments

What this PR does / why we need it:

the gavy/httpexpect library is not maintained and this PR raises a simpler alternative solution for the library. The idea is to keep the same interface and contract, so tests can be kept with the same behavior. This is a starter PR and the "library" will be extended as more tests starts to use it.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Documentation only

Which issue/s this PR fixes

fixes https://github.com/kubernetes/ingress-nginx/issues/8722

How Has This Been Tested?

Local testing has been made filtering only the changed folder FOCUS="\[Default Backend\]" make e2e-test

SS
------------------------------
• [SLOW TEST:14.680 seconds]
[Default Backend] SSL should return a self generated SSL certificate
/go/src/k8s.io/ingress-nginx/test/e2e/defaultbackend/ssl.go:29
------------------------------
• [SLOW TEST:19.995 seconds]
[Default Backend] should return 404 sending requests when only a default backend is running
/go/src/k8s.io/ingress-nginx/test/e2e/defaultbackend/default_backend.go:32
------------------------------
• [SLOW TEST:23.911 seconds]
[Default Backend] change default settings should apply the annotation to the default backend
/go/src/k8s.io/ingress-nginx/test/e2e/defaultbackend/with_hosts.go:38
------------------------------
• [SLOW TEST:26.438 seconds]
[Default Backend] custom service uses custom default backend that returns 200 as status code
/go/src/k8s.io/ingress-nginx/test/e2e/defaultbackend/custom_default_backend.go:36
------------------------------
 SUCCESS! 26.4771346s

Checklist:

  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I've read the CONTRIBUTION guide
  • [x] I have added tests to cover my changes.
  • [x] All new and existing tests passed.

knabben avatar Jul 24 '22 15:07 knabben

/area stabilization /triage accepted /priority important-soon /kind feature /ok-to-test

strongjz avatar Jul 25 '22 17:07 strongjz

@knabben can you do a rebase and see if that fixes the conflicts?

Also fair point on the function names, can you open an issue to make sure it is being tracked?

strongjz avatar Aug 09 '22 16:08 strongjz

/test pull-ingress-nginx-test

strongjz avatar Aug 09 '22 18:08 strongjz

@knabben looks like some tests are using deprecated Ginko functionality. Can you go through and update them for Ginko v2?

strongjz avatar Aug 09 '22 18:08 strongjz

Since most of the code was ported from the library, I think makes sense to add a note for the issue/repo URL in the folder.

knabben avatar Sep 01 '22 02:09 knabben

/assign @rikatz

knabben avatar Sep 01 '22 13:09 knabben

/retest

knabben avatar Sep 04 '22 20:09 knabben

/lgtm /approve Just need to rebase and fix the build/build.sh

Thank you for this one!!

rikatz avatar Sep 05 '22 01:09 rikatz

/lgtm 🥳

rikatz avatar Sep 05 '22 10:09 rikatz

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knabben, rikatz

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 avatar Sep 05 '22 10:09 k8s-ci-robot