-
Notifications
You must be signed in to change notification settings - Fork 134
Add invariant tests #4326
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?
Add invariant tests #4326
Conversation
|
Commit: 73a1501
15 interesting tests: 7 KNOWN, 6 SKIP, 1 RECOVERED, 1 flaky
Top 28 slowest tests (at least 2 minutes):
|
63ee262 to
a195594
Compare
| no_drift test checks that there are no actions planned after successful deploy. If that's not the case, the | ||
| test will dump full JSON plan to the output. | ||
|
|
||
| In order to add a new test, add a config to configs/ and include it in test.toml. |
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.
The ones in this PR are specific to bundles. Will they generalize? If not, they can be moved the bundles dir.
| @@ -0,0 +1,39 @@ | |||
| #!/usr/bin/env python3 | |||
| """ | |||
| Check that all actions in plan are "skip". | |||
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.
Maybe verify_no_drift.py?
| foo: | ||
| warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID | ||
| display_name: test-alert-$UNIQUE_NAME | ||
| file_path: ./alert.dbalert.json |
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.
@shreyas-goenka Do we need one with the alert inlined?
| cat LOG.deploy | contains.py '!panic' '!internal error' > /dev/null | ||
|
|
||
| # Special message to fuzzer that generated config was fine. | ||
| # Any failures after this point will be considered as "bug detected" by fuzzer. |
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.
Fuzzer == vary over input configs?
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.
Yes
39b9307 to
3de7df5
Compare
Add 9 minimal resource configuration templates to test the no-drift invariant across different resource types (jobs, pipelines, experiments, models, volumes, schemas, clusters, model serving endpoints). Changes: - Add configs/ directory with minimal .yml.tmpl files for each resource type - Add data/ directory for supporting files (pipeline.py) - Update no_drift/script to copy data files at test start - Update test.toml to include new configs and ignore *.py files - Update out.test.toml with expanded config matrix All configs tested and verified on both testserver and AWS cloud with direct and terraform deployment engines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add 4 more resource configurations to the no_drift invariant test, bringing the total to 13 working configs. New configs: - alerts (with .dbalert.json file) - dashboards (with .lvdash.json file) - apps (with source_code_path directory) - secret_scopes Also created sql_warehouse.yml.tmpl but excluded from test due to drift issues. Changes: - Add 5 new config templates in configs/ - Add 3 new data files in data/ (alert.dbalert.json, dashboard.lvdash.json, app/) - Update test.toml to include new configs and ignore *.json and app directory - Update out.test.toml with expanded config matrix All 13 configs tested on testserver with both direct and terraform engines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Complete the no_drift invariant test coverage by adding the remaining resource types, bringing the total to 17 working configs. New configs: - quality_monitor (with dependent schema) - database_instance - database_catalog (with dependent database_instance) - synced_database_table (with dependent instance and catalog) Changes: - Add 3 new config templates in configs/ - Fix quality_monitor to use $CURRENT_USER_NAME - Update test.toml with 4 additional configs - Update out.test.toml with full config matrix All 17 configs tested successfully on testserver with both direct and terraform deployment engines, showing zero drift after deploy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
App names must be between 2-30 characters. Changed from 'test-app-' to 'app-' to fit within the limit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
3de7df5 to
10b8a2f
Compare
Changes
Add a new kind of test - invariant test for DABs (a subtype of acceptance test).
Invariant tests are acceptance tests that can be run against many configs to check for certain properties.
Unlike regular acceptance tests full output is not recorded, unless the condition is not met.
In this PR there is one invariant test added - no_drift, that checks that there are no actions planned after successful deploy. If that's not the case, the test will dump full JSON plan to the output.
Why
Simplify resource implementation testing - implementors only need to provide a config to get all invariants check.
With no_drift in particular, we've had several cases where this condition is not met and it's still the case for some configurations of permissions and grants. It's also going to become more important as we replace "server_side_default" with something more precise.
The other use of invariant tests is fuzzing - since they work on any config, they can be used on randomly generated configs to find bugs.