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 the path-related test case to the App Gateway test suite #14762

Closed
wants to merge 36 commits into from

Conversation

mvshao
Copy link
Contributor

@mvshao mvshao commented Jul 6, 2022

Description

Changes proposed in this pull request:

  • Add the path-related test case to the Application Gateway test suite

Related issue(s)

See the issue here - #14654

VOID404 and others added 30 commits June 7, 2022 09:35
Needs some chart cleanup.
Test job will fail because it doesn't wait for test app to deploy.
Not yet finished!
Problem was caused by Istio
in Dockerfile
Https can't be used due to a bug in gateway
There is some regression in features
and tests don't pass
Now makefile waits for pod to run, then displays logs and exits when
tests are completed
@mvshao mvshao added area/application-connector Issues or PRs related to application connectivity do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. area/tests Issues or PRs related to tests labels Jul 6, 2022
@mvshao mvshao self-assigned this Jul 6, 2022
@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 6, 2022
@kyma-bot
Copy link
Contributor

kyma-bot commented Jul 6, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kyma-bot kyma-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 6, 2022
@netlify
Copy link

netlify bot commented Jul 6, 2022

🥰 Documentation preview ready! 🥰

Name Link
🔨 Latest commit 9a50cb2
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/62c5705734715b0008b565b5
😎 Deploy Preview https://deploy-preview-14762--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

}
var actualCode, expectedCode int

switch service.Name {
Copy link
Contributor

Choose a reason for hiding this comment

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

The switch-case statement is redundant as the code for each case is identical.

v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func (gs *GatewaySuite) TestPath() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we don't need that code at all. Please, notice the following:

  • The test cases description is contained within Application CRD
  • The expected response code is contained within Application CRD
  • Test cases checking positive authorisation are similar to path related ones in that sense that the Go code executing it is identical.

So there are two possibilities:

  • Have one Application CRD with test cases for positive authorisation and path related cases
  • Have a set of Go functions and a set of Application CRDs and combine them. We need the following code:
    • Executing Get request and checking response code
    • Executing Post request with body, additional query parameters and headers
    • ...

@mvshao mvshao closed this Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application-connector Issues or PRs related to application connectivity area/tests Issues or PRs related to tests do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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