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

Don't export ValidationSplitWebhook test #10394

Merged
merged 7 commits into from
Nov 23, 2024

Conversation

sheidkamp
Copy link

@sheidkamp sheidkamp commented Nov 22, 2024

Description

Create a new SuiteRunner for the ValidationTests which does not include the split validation test, which
fails when imported by other projects. k8sgateway#10374 created to
track a fix that will allow tests that update Helm values to be exported.

Can't use Skip in the tests because the e2e.MustTestHelper(ctx, testInst) command in the suite constructor fails due to not finding a locally built gloo helm chart

This test should be safe to skip because it is not testing any EE or Helm specific functionality, and is more of an integration/smoke test for the split webhook. The Helm upgrades performed are to update webhook values, not to test anything Helm related.

Testing

Local tests - ValidationSplitWebhook tests are still running: https://github.com/solo-io/gloo/actions/runs/11974993999/job/33390398462#step:7:14910

In EE:
Pull in OSS branch and run:

go clean -testcache; GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore go test -v -timeout 600s ./test/kubernetes/e2e/tests -run  ^TestOssFeatures$/^OssValidationStrict$

and get results:

--- PASS: TestOssFeatures (136.74s)
    --- PASS: TestOssFeatures/OssValidationStrict (136.74s)
        --- PASS: TestOssFeatures/OssValidationStrict/ValidationStrictWarnings (39.56s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationStrictWarnings/TestInvalidUpstreamMissingPort (17.09s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationStrictWarnings/TestRejectsInvalidVSMissingUpstream (0.16s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationStrictWarnings/TestVirtualServiceWithSecretDeletion (22.31s)
        --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid (25.18s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestRejectTransformation (2.42s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestRejectsInvalidGatewayResources (0.15s)
            --- SKIP: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestRejectsInvalidRatelimitConfigResources (0.00s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestRejectsInvalidVSMethodMatcher (0.17s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestRejectsInvalidVSTypo (0.15s)
            --- PASS: TestOssFeatures/OssValidationStrict/ValidationRejectInvalid/TestVirtualServiceWithSecretDeletion (22.29s)
PASS

(TestOssFeatures/OssValidationStrict/ValidationSplitWebhook is not present)

@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/7256

@sheidkamp sheidkamp enabled auto-merge (squash) November 22, 2024 17:07
// that are not valid when the project is imported as a helm dependency
// https://github.com/k8sgateway/k8sgateway/issues/10374 has been created to create a fix for this.
// If more tests are added that depend on the helm chart/values/helpers, the above issue should be resolved instead of using this approach
func ValidationStrictSuiteRunnerForExport() e2e.SuiteRunner {
Copy link

Choose a reason for hiding this comment

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

why not leave the exported one ValidationStrictSuiteRunner so s-p import doesn't need to change?

Copy link

Choose a reason for hiding this comment

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

and the new one can be ValidationStrictSuiteRunnerNotExported or something?

Copy link

Visit the preview URL for this PR (updated for commit 060d445):

https://gloo-edge--pr10394-sheidkamp-dont-expor-ijctzmml.web.app

(expires Sat, 30 Nov 2024 04:11:30 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@sheidkamp sheidkamp merged commit aac1d58 into main Nov 23, 2024
19 checks passed
@sheidkamp sheidkamp deleted the sheidkamp/dont-export-split-validation-test branch November 23, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants