-
Notifications
You must be signed in to change notification settings - Fork 93
Integration tests #929
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
Integration tests #929
Conversation
52dc07e to
47c0a91
Compare
0cb6c07 to
6cc1993
Compare
gehrkefc
left a comment
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.
1 comment, but this is too good for me to request any changes. Great work!
d4f0e0b to
58ee69a
Compare
| paramCodec = runtime.NewParameterCodec(paramScheme) | ||
| // Please keep the gvkKey entries in alphabetical order, on a field-by-field basis | ||
| typeSpecificIndexedFields = map[string][][]string{ | ||
| TypeSpecificIndexedFields = map[string][][]string{ |
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'm thinking of moving this to an option in the CachefactoryOption struct so we can modify things per-gvk (fields, type hints, etc). But that would be in another PR. For now, the only other way I figured was making this public for the sake of modifying it in the tests. Not great, but does the job. Open to other suggestions though
| # - schemaID: columns.cattle.io.namespacedsomecolumns | ||
| # expected: | ||
| # - [foo, $timestamp, $timestamp] | ||
| --- |
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.
It's also possible to have manifests directly in the .test.yaml file. IS this a good idea or should we force an external manifest file?
|
Some recent changes (cc: @gehrkefc ) Was missing some HTTP response code check in two of those tests, move the tests to |
ericpromislow
left a comment
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.
Lots going on here but it works, and looks fine.
Issue rancher/rancher#48080
This PR adds an integration test harness for Steve that allows us to tests the Steve API with everything running (schema controllers, formatters, virtual func transformation, etc) the same way it runs within Rancher or within the Steve binary.
We can then avoid doing the setup code for steve like we're doing in pkg/sqlcache/integration_test.go. In fact, we're likely to get rid of that test and move the tests over to this new harness (out of scope for this PR).
The only dependency for running integration tests is a Kubernetes cluster (eg:
k3d). If aKUBECONFIGis not provided then the test entrypoint will try to create one usingk3d.How to use
See tests/README.md, but the gist is:
make integration-tests.Features
INTEGRATION_TEST_DEBUG=true. You can then curl the running steve API, inspect the Kubernetes cluster and explore the SQLite database.*.manifests.yamlfiles (which you can directly apply viakubectlwhen troubleshooting) and*.test.yamlfiles which defines the tests (eg: filter params, expected output).The goal of this PR is NOT to bring a LOT of tests. Instead, I'm adding a few examples and then future PRs can add more tests. This will make it easier to get the test harness reviewed.
Note: It's still possible to write tests not using YAML if those tests don't really match that.
How to review
Best to review commit by commit. The first commit introduces the test harness: no tests, just utilities for building the tests in the next few commits.