-
Notifications
You must be signed in to change notification settings - Fork 139
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
chore: test suite refactor #2289
base: main
Are you sure you want to change the base?
Conversation
- Pulls logic for defaulting to active namespace (K8S) moved UP to CLI during flag default calculation. - Pushes logic of deciding between f.Namespace vs f.Deploy.Namespace down into implementations. - Updates some tests which needed to have their environment cleared. - Refactors Pipelines tests to use client API. - Removes namespaces as a state variable all structures, instead passing as an argument. - Moves FromTempDirectory to testing package for use outside cmd.
- Utilize the `make check-embedded-fs` target - Renames schema-check to check-schema to match other checks - Delegates runtime and builder matrix logic to test suite impl - Relocates converage reformatting to workflows
- Adds test-all target - integration and e2e tests clearly state they run unit tests too - integration and e2e tests clear cache - Adds a check-embedded-fs target - Adds several missing .PHONY lines
- Default builder and pusher set to embedded Host Builder/Pusher(oci) - Most tests clear environment - Environment defaults can be controlled via environment variables - Tests which require back-compat `git` binary actively check and skip when running with a cleared environment (both integration and unit). - Bugfixes for when run in tandem with E2E tests - Ignores go-created directories in the default home path (testdata)
- Updates docs and moves package doc to a README.md - Sets up MatrixRuntimes and MatrixBuilders as an opt-in filter - Implements delete - Bugfixes when run via make
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lkingland 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 |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
||
test-unit: |
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.
I would not recommend deleting these.
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.
Will make test-all
run really all tests? What about coverage have you ensured that there are no conflicts?
@@ -1,10 +1,10 @@ | |||
name: Func Unit Test |
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.
I do not get this rename. This is foremost "unit test". It also runs "template test" but that is IMO secondary.
go test -ldflags "$(LDFLAGS)" -cover -tags "e2e" -timeout 30m --coverprofile=coverage.txt ./... | ||
|
||
.PHONY: test-e2e | ||
test-all: func-instrumented ## Run unit+integration+E2E tests (cluster required) |
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.
Does this properly gather coverage from the instrumented func?
.PHONY: test-e2e | ||
test-all: func-instrumented ## Run unit+integration+E2E tests (cluster required) | ||
go clean -testcache | ||
go test -ldflags "$(LDFLAGS)" -cover -tags "integration e2e" -timeout 60m --coverprofile=coverage.txt ./... |
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.
does not this miss "on cluster build" tag?
This Pull Request is stale because it has been open for 90 days with |
This PR provides the general structure for an upgrade to the testing system (both integration and E2Es) relying on the newly created Host builder for core functionalit, along with general simplification.
Currently a WIP.
/kind cleanup