-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: move e2e test content from literals to files #14725
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alan Clucas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors end-to-end tests by moving inline YAML content from test files to separate files and updating the test framework to only accept file-based workflows. The goal is to reduce file sizes and improve test maintainability by consolidating workflow definitions in dedicated directories.
- Moved hundreds of inline YAML workflow definitions to external files in
test/e2e/functional/
,test/e2e/executor/
,test/e2e/cron/
, andtest/e2e/corefunctional/
directories - Modified the
Given.Workflow()
method and related functions to only accept file paths with@
prefix, removing support for literal YAML strings - Updated all test files to use the new file-based approach with
@filename.yaml
notation instead of embedded YAML
Reviewed Changes
Copilot reviewed 136 out of 136 changed files in this pull request and generated 15 comments.
Show a summary per file
File | Description |
---|---|
test/e2e/workflow_test.go | Updated workflow test calls to use external YAML files instead of inline definitions |
test/e2e/workflow_template_test.go | Converted workflow template tests to use file references |
test/e2e/workflow_inputs_orverridable_test.go | Refactored input override tests to use external workflow files |
test/e2e/workflow_configmap_substitution_test.go | Updated configmap substitution tests to reference external files |
test/e2e/suspend_test.go | Converted suspend node tests to use file-based workflows |
test/e2e/retry_test.go | Updated retry strategy tests to use external YAML files |
test/e2e/resource_template_test.go | Refactored resource template tests to use file references |
test/e2e/pod_cleanup_test.go | Converted pod cleanup tests to use external workflow definitions |
test/e2e/hooks_test.go | Updated lifecycle hook tests to use file-based approach |
test/e2e/functional_test.go | Refactored functional tests to reference external YAML files |
test/e2e/failed_main_test.go | Updated failed main test to use external file reference |
test/e2e/daemon_pod_test.go | Converted daemon pod tests to use file-based workflows |
test/e2e/cron_test.go | Updated cron workflow tests to use external YAML files |
test/e2e/fixtures/given.go | Modified to only accept file paths, removed support for literal YAML strings |
test/e2e/functional/*.yaml | New workflow definition files extracted from inline YAML |
test/e2e/executor/*.yaml | New executor-specific workflow files |
test/e2e/cron/*.yaml | New cron workflow definition files |
test/e2e/corefunctional/*.yaml | New core functional workflow files |
name: cmref-parameters | ||
key: cmref-key | ||
`). | ||
Workflow("@functional/workflow-inputs-overridable-wf.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is calling the same workflow file as the previous test but expects different behavior. The test creates different ConfigMaps but uses the same workflow file, which may not contain the correct configuration for this specific test case.
Workflow("@functional/workflow-inputs-overridable-wf.yaml"). | |
Workflow("@functional/workflow-inputs-overridable-wf-args-valuefrom.yaml"). |
Copilot uses AI. Check for mistakes.
- name: message | ||
value: input-value | ||
`). | ||
Workflow("@functional/workflow-inputs-overridable-wf.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file as the previous tests but expects different behavior. Each test scenario should use a distinct workflow file that matches its specific test requirements.
Workflow("@functional/workflow-inputs-overridable-wf.yaml"). | |
Workflow("@functional/workflow-inputs-overridable-wf-3.yaml"). |
Copilot uses AI. Check for mistakes.
- echo | ||
- "{{inputs.parameters.message}}" | ||
`). | ||
Workflow("@functional/workflow-template-configmapkeyselector-wf.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file as the previous test but expects different behavior (name substitution vs key substitution). Each test scenario should use a distinct workflow file that matches its specific test case.
Workflow("@functional/workflow-template-configmapkeyselector-wf.yaml"). | |
Workflow("@functional/workflow-template-configmapnameselector-wf.yaml"). |
Copilot uses AI. Check for mistakes.
- echo | ||
- "{{inputs.parameters.message}}" | ||
`). | ||
Workflow("@functional/workflow-template-configmapkeyselector-wf.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file as the previous tests but expects different behavior (invalid parameter substitution). The workflow file should match the specific test scenario.
Workflow("@functional/workflow-template-configmapkeyselector-wf.yaml"). | |
Workflow("@functional/workflow-template-configmapkeyselector-wf-invalid.yaml"). |
Copilot uses AI. Check for mistakes.
command: ["/bin/sh", "-c"] | ||
args: ["/bin/sleep 1; /argosay"] | ||
`).When(). | ||
Workflow("@functional/lifecycle-hook.yaml").When(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file as the success test but expects different behavior (failure scenario). The workflow should be configured specifically for testing failure hooks.
Workflow("@functional/lifecycle-hook.yaml").When(). | |
Workflow("@functional/lifecycle-hook-fail.yaml").When(). |
Copilot uses AI. Check for mistakes.
container: | ||
image: argoproj/argosay:v2 | ||
`). | ||
Workflow("@functional/test-pod-cleanup-on-pod-completion-label-selected.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file for both failed and succeeded pod scenarios with label selection but expects different behavior. Each scenario should use a distinct workflow file.
Workflow("@functional/test-pod-cleanup-on-pod-completion-label-selected.yaml"). | |
Workflow("@functional/test-pod-cleanup-on-pod-completion-label-selected-succeeded.yaml"). |
Copilot uses AI. Check for mistakes.
Workflow("@functional/test-pod-cleanup-on-pod-success.yaml"). | ||
When(). | ||
SubmitWorkflow(). | ||
WaitForPod(fixtures.PodCompleted) | ||
}) | ||
s.Run("SucceededPod", func() { | ||
s.Given(). | ||
Workflow(` | ||
metadata: | ||
generateName: test-pod-cleanup-on-pod-success- | ||
spec: | ||
podGC: | ||
strategy: OnPodSuccess | ||
entrypoint: main | ||
templates: | ||
- name: main | ||
container: | ||
image: argoproj/argosay:v2 | ||
`). | ||
Workflow("@functional/test-pod-cleanup-on-pod-success.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file for both failed and succeeded pod scenarios but expects different behavior. Each scenario should use a distinct workflow file configured appropriately.
Copilot uses AI. Check for mistakes.
Workflow("@functional/test-pod-cleanup-on-pod-success-label-match.yaml"). | ||
When(). | ||
SubmitWorkflow(). | ||
WaitForPod(fixtures.PodCompleted) | ||
}) | ||
s.Run("SucceededPod", func() { | ||
s.Given(). | ||
Workflow(` | ||
metadata: | ||
generateName: test-pod-cleanup-on-pod-success-label-match- | ||
spec: | ||
podGC: | ||
strategy: OnPodSuccess | ||
labelSelector: | ||
matchLabels: | ||
evicted: true | ||
entrypoint: main | ||
templates: | ||
- name: main | ||
container: | ||
image: argoproj/argosay:v2 | ||
metadata: | ||
labels: | ||
evicted: true | ||
`). | ||
Workflow("@functional/test-pod-cleanup-on-pod-success-label-match.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file for both failed and succeeded pod scenarios with label matching but expects different behavior. Each scenario should use a distinct workflow file.
Copilot uses AI. Check for mistakes.
name: test-exit-handler | ||
`). | ||
WorkflowTemplate("@corefunctional/test-exit-handler.yaml"). | ||
Workflow("@corefunctional/test-exit-handler.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using the same workflow file name for both WorkflowTemplate and Workflow, but they likely need different configurations. The workflow should reference the workflow template, not be identical to it.
Workflow("@corefunctional/test-exit-handler.yaml"). | |
Workflow("@corefunctional/test-lifecycle-hook.yaml"). |
Copilot uses AI. Check for mistakes.
value: "hello world" | ||
`). | ||
WorkflowTemplate("@corefunctional/test-exit-handler.yaml"). | ||
Workflow("@corefunctional/test-lifecycle-hook.yaml"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is using a different workflow file than the WorkflowTemplate defined above. The workflow should be compatible with and reference the 'test-exit-handler' template.
Workflow("@corefunctional/test-lifecycle-hook.yaml"). | |
Workflow("@corefunctional/test-exit-handler.yaml"). |
Copilot uses AI. Check for mistakes.
Part of #13610
Motivation
The .go files are too big.
Vibe coded a tool to move these, so hopefully this is sane and the content is necessarily identical.
Modifications
Remove literal kubernetes objects from e2e tests and put them in files.
Only accept files from now on.
Verification
CI will tell us if it's horridly broken
Documentation
Developer experience only