-
Notifications
You must be signed in to change notification settings - Fork 224
PyDABs: acceptance-test authoring skill + per-resource coverage guard #6527
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
Changes from all commits
a21e45d
687cece
8620d03
1df03bb
81d29de
43de1d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| description: Rules for authoring PyDABs resource acceptance tests | ||
| globs: acceptance/bundle/python/** | ||
| paths: | ||
| - "acceptance/bundle/python/**" | ||
| --- | ||
|
|
||
| **RULE: Before adding a PyDABs resource acceptance test, read `acceptance/bundle/python/README.md`.** It covers the `<plural>-support/` fixture layout, how to source and adapt realistic field values, the version/engine `test.toml` knobs, and the determinism re-run. Every PyDABs resource needs one (enforced by `test_python_support_coverage`). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../.agents/rules/pydabs-acceptance-tests.md |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # PyDABs resource acceptance tests | ||
|
|
||
| Each `<plural>-support/` directory is the acceptance test for one PyDABs resource. It | ||
| checks that the resource loads both from YAML and from Python and that a mutator runs | ||
| over it. `test_python_support_coverage` | ||
| (`python/databricks_tests/core/test_python_support.py`) requires every PyDABs resource | ||
| to have one, so a newly-onboarded resource needs a fixture here. | ||
|
|
||
| Copy an existing one — `alerts-support/` (a resource with required nested fields) or | ||
| `catalogs-support/` (direct-engine only) are the canonical examples. A fixture is six | ||
| files: | ||
|
|
||
| - `databricks.yml` — `bundle.name: my_project`, `sync: {paths: []}`, a top-level | ||
| `python:` block wiring `resources:load_resources` + `mutators:update_<singular>`, and | ||
| one YAML-declared instance `<plural>.my_<name>_1`. | ||
| - `resources.py` — `load_resources()` adds a second instance `my_<name>_2` via | ||
| `resources.add_<singular>(...)`. | ||
| - `mutators.py` — a `@<singular>_mutator` that appends `" (updated)"` to a required | ||
| string field; it runs over both instances. | ||
| - `script` — copy it verbatim (`bundle validate --output json | jq "pick(...)"`). | ||
| - `test.toml` — `Cloud = false`. | ||
| - `output.txt` — generated, never hand-written. | ||
|
|
||
| ## Authoring a new one | ||
|
|
||
| 1. Confirm the resource is wired: `python/databricks/bundles/<plural>/` exists, and | ||
| `add_<singular>` / `<singular>_mutator` are in `databricks.bundles.core`. If not, it | ||
| must be onboarded in PyDABs first. | ||
| 2. Required fields are the `VariableOr[...]` (no default) fields in | ||
| `python/databricks/bundles/<plural>/_models/<singular>.py`; set all of them, | ||
| recursing into required nested objects. `VariableOrOptional[...] = None` fields are | ||
| optional — omit them. | ||
| 3. Get realistic values from `acceptance/bundle/invariant/configs/<singular>.yml.tmpl`, | ||
| but **adapt**: replace `$UNIQUE_NAME` / `$TEST_DEFAULT_WAREHOUSE_ID` and other `$VAR`s | ||
| with plain literals, and drop cloud-only blocks (`permissions`, `grants`, | ||
| `file_path`) — this test is local and deterministic. | ||
| 4. `test.toml`: add `EnvMatrix.PYDAB_VERSION = ["current"]` for a brand-new resource | ||
| (it only exists in the current wheel), and `EnvMatrix.DATABRICKS_BUNDLE_ENGINE = | ||
| ["direct"]` for a direct-only resource. | ||
| 5. Generate the golden: | ||
| `go test ./acceptance -run 'TestAccept/bundle/python/<plural>-support' -update`. | ||
| 6. **Re-run without `-update`** — it must pass against the golden you just generated. A | ||
| test that only passes with `-update` is nondeterministic (usually a `$VAR` or a | ||
| volatile field left in); fix it before finishing. | ||
|
|
||
| Note: `bundle validate` normalizes the `python:` key to `experimental.python` in the | ||
| output — that's expected. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| """Coverage guard: every PyDABs resource must have an acceptance fixture. | ||
|
|
||
| Asserts each resource in the _ResourceType registry has an | ||
| acceptance/bundle/python/<plural>-support/ fixture (see that directory's README.md for | ||
| how to author one); this fails CI until it exists. | ||
| """ | ||
|
|
||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from databricks.bundles.core._resource_type import _ResourceType | ||
|
|
||
| _ACCEPTANCE_DIR = Path(__file__).parents[3] / "acceptance" / "bundle" / "python" | ||
|
|
||
| # Resources knowingly lacking a <plural>-support fixture. Shrink-only: the test fails | ||
| # if an entry here is actually covered, so gaps can only close. | ||
| _LACKING = { | ||
| # jobs predates the <plural>-support convention; covered across the suite instead. | ||
| "jobs", | ||
|
Comment on lines
+19
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you going to migrate the test for this resource?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I plan on doing it eventually, but I am focussing on completing the codegen for now, the migration can be done later on as well |
||
| } | ||
|
|
||
| _PLURALS = sorted(t.plural_name for t in _ResourceType.all()) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("plural", _PLURALS) | ||
| def test_python_support_coverage(plural: str): | ||
| covered = (_ACCEPTANCE_DIR / f"{plural}-support" / "databricks.yml").exists() | ||
|
|
||
| if plural in _LACKING: | ||
| assert not covered, f"{plural!r} now has a fixture; remove it from _LACKING" | ||
| else: | ||
| assert covered, ( | ||
| f"no acceptance/bundle/python/{plural}-support/ fixture for {plural!r}; " | ||
| "add one (see acceptance/bundle/python/README.md) or add it to _LACKING" | ||
| ) | ||
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.
.agents/rules/*.mdalso need to be symlinked to.cursor/rules/<rule>.mdc. would be great if you could write a lint rule that auto-adds these symlinks 🙏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.
added, the rule. Please TAL once