Auto test every field mutation - #6452
Draft
denik wants to merge 141 commits into
Draft
Conversation
denik
changed the title
Collaborator
Integration test reportCommit: 0528be3
6 interesting tests: 4 FAIL, 1 RECOVERED, 1 flaky
Top 50 slowest tests (at least 2 minutes):
|
denik
force-pushed
the
denik/extensive-test
branch
4 times, most recently
from
September 11, 2026 09:26
04d32c3 to
d34b72b
Compare
Recreating a resource re-keys the parent but not its permissions/grants child: the child's state entry keeps the old id, DoRead echoes it back, and the planner re-proposes the same update on every deploy. The ACL itself is applied to the recreated object, so this is stale bookkeeping rather than lost access, and terraform converges -- hence the per-engine plan files. Co-authored-by: Isaac
model_with_permissions.yml.tmpl was model.yml.tmpl plus a permissions block, so the two tested the same resource twice. Invariant configs carry permissions or grants by default; a permissions-free variant is not a separate case. continue_293 now excludes the model config outright, since model permissions did not work until 0.297.0. Co-authored-by: Isaac
Moves every field of every single-resource invariant config through each ordered pair of a small value set -- absent included, so add and remove are just the pairs with absent on one side -- and records whether the plan proposed anything, the apply succeeded, and the next plan was clean. Runs in-process against CalculatePlan/Apply rather than shelling out to bundle deploy, so 5502 observations across 22 resource types take 35s instead of being dominated by bundle file sync. Permissions and grants are stripped first; a bad outcome does not fail the test, it lands in out.<resource_type>.fields.txt, and the committed goldens are what turn an SDK bump into a visible diff. Co-authored-by: Isaac
Every field of catalog.UpdateSchema is omitempty and DoUpdate forwards only the config's own ForceSendFields, so removing the comment sends an empty PATCH body and UC keeps the old value. The deploy reports success and every later plan re-proposes the same update. Co-authored-by: Isaac
The earlier approach: "# ACTION:" annotations in the invariant configs drive apply_update and apply_remote_update, which plan, apply and re-plan one field at a time through the full CLI. apply_update is now covered far more thoroughly by bundle/direct/dresources_test. apply_remote_update is not -- it drifts the remote by restoring a state snapshot, which the Go catalog does not do yet -- so this is kept for that half. Drop the commit if the remote case lands there instead. Co-authored-by: Isaac
A harness rebuilt inside a field-level subtest goes on being used by later fields, but it captured that subtest's t.Context(), which is cancelled the moment the subtest returns -- 56 observations came back as "context canceled" from the SDK rate limiter rather than a real verdict. Thread the resource-type context in instead. With those unmasked, model serving telemetry turned out to be rejected 36 times: the block only validates against a catalog and schema that exist and are writable. Skip the subtree, which needed skip keys to support a "<prefix>.*" form. Co-authored-by: Isaac
…text cloud: true was the wrong default -- a resource type should be covered everywhere, and the expensive ones still have to be covered. Replaced with slow: true, which only drops a type under -short: the PR cloud leg skips them, the nightly does not. Files that said nothing but cloud: true are gone. t.Context() is cancelled just before Cleanup functions run, so every harness destroy was issuing its API calls on a dead context and silently leaking the resource; a harness rebuilt inside a field subtest hit the same thing once that subtest returned. context.WithoutCancel keeps the values and drops the deadline. Reports move to output/, so the package directory reads as seven entries instead of thirty. Co-authored-by: Isaac
It drives CalculatePlan and Apply, so it tests the engine rather than any one resource implementation, and it belongs beside the engine instead of under dresources. The old dresources_test directory name also misapplied Go's _test package-name convention to a directory. Value libraries move to testdata/fields, which the go tool ignores by name. Goldens stay in output/. Every file in the package is a _test.go, so GoFiles is empty and none of this reaches the binary. Co-authored-by: Isaac
The committed output/<type>.txt now lists just the results worth looking at, one line each, with from and to as their own columns. output/<type>.full.txt keeps everything plus the summary and the not-covered list, and is gitignored since it moves whenever a passing row does. 304 findings out of 5442 observations. A SUPPRESSED whose reason is "empty" is not one of them: both sides were empty, so nothing the user expressed was lost. DRIFT becomes POST_DEPLOY_DRIFT, which is what it measures. Transition subtests are named absent_to_true rather than with an arrow, so a single case re-runs without shell quoting, and now -v prints the post-deploy plan behind any finding. Dropped the slow flag: every resource type runs everywhere. Renaming an app is a real operation, so it is no longer skipped for being slow. It records DEPLOY_ERROR "timed out: app is deleting", which is the finding. A per-operation deadline backs that up with a TIMEOUT verdict. Main fixed clearing a schema comment, so that test becomes clear_field: it now asserts comment converges and records that custom_max_retention_hours does not. Co-authored-by: Isaac
They were committed by accident: output/<type>.txt is the file worth diffing. Co-authored-by: Isaac
They were committed by accident: output/<type>.txt is the file worth diffing. Co-authored-by: Isaac
Slices and maps are covered two ways. The container becomes a field whose values are the config's own and that value minus its last entry, so with absent in the set one field covers adding and removing an entry as well as the whole container -- all with data the backend already accepted. Separately, a pattern like tasks[*].description is expanded against the deployed config to the indices that exist, so fields inside an element are tested like any other. A pattern with nothing behind it is reported as not covered rather than silently passing. base: in a value library seeds a block the invariant config does not declare, so its fields become reachable. A job's git_source needs a provider, a url and exactly one ref, and cannot be built up one field at a time from nothing; seeded, its skip list drops from nine entries to the two that are genuinely a different side of the same oneof. Value pairs are now walked as a single chain instead of staged one at a time. The values form a complete digraph, so one Eulerian circuit covers every ordered pair exactly once and each move starts where the last ended -- one deploy per transition instead of two, which took jobs from 381s to 233s. Walk order is shuffled from a seed derived from the field path: fixed per field so goldens stay byte-stable, different between fields so the suite is not always taking the same path. TestTransitionsCoverEveryPairInOneChain is what guarantees nothing is left for a second chain to pick up. Configs of a resource type now run in parallel, so the report is guarded. out.full.txt indents the evidence under each finding: the post-deploy plan for drift, the whole API error for a rejection. Co-authored-by: Isaac
One config per resource type -- the simplest one -- and whatever structure that config lacks is seeded through testdata/fields instead. A slice or map is a field in its own right, whose values are the config's own and that value minus its last entry, so with absent in the set one field covers adding and removing an entry as well as the whole container. Patterns like tasks[*].description expand against the deployed config, so fields inside an element are tested like any other, and what no config declares is named rather than passed over. Findings are classified until each verdict names one mechanism: UPDATE_IGNORED (write accepted, remote unmoved on two reads) separated from STALE_READ (unmoved on the first read only), COLLATERAL_DRIFT (updating this field left another one drifting -- 119 rows were one field wearing other fields' names), BASELINE_DRIFT (drifts with nothing changed), and OK_INERT / INERT_NOT_HONOURED, which check the ignore_local_changes declarations rather than trusting them. Order comes from git HEAD so a finding cannot be retried away, and the pairs are walked as one Eulerian circuit, halving the deploys. Three bugs in the harness, each of which quietly under-reported coverage: map wildcards never expanded, since splitPattern emitted the wildcard twice; the not-covered list was a union across configs, so it named fields other configs were testing; and neither a map key nor a slice element could be removed, since dyn's path parser reads brackets as indices and deletePath only handled keys. libs/testserver grows SettleAsyncImmediately, off by default so acceptance keeps exercising the waiters. The SDK's poller has a hardcoded one-second backoff with no exported way to shorten it, so one in-progress read per update cost serving endpoints more wall time than every other resource type combined: 512s to 5s. 7052 observations over 22 resource types in 76s. Co-authored-by: Isaac
The generic "x"/"y" defaults are rejected outright on a real workspace for a field that names another object, and the retention period is validated in days, so those fields reported nothing but BACKEND_ERROR on cloud. Co-authored-by: Isaac
The suite now writes values through libs/structs (structaccess over the resource's own Go type, structpath for paths) and syncs the result into the dynamic tree the way a mutator does, instead of manipulating dyn.Value directly. Nothing in the package depends on libs/dyn any more. That also removes every hand-written substitute for something dyn lacked: a path parser that could hold a map key, a delete-by-path, a parent-creating walk. And it makes "absent" the distinction the API actually sees -- the zero value with the field out of ForceSendFields -- so a list element or map entry now becomes settable where it previously reported UNSETTABLE: 91 of those are gone and the suite records 252 more observations, in a third of the wall time. The order seed is a constant now rather than HEAD. Some verdicts depend on the order -- a field the API cannot clear leaves the remote holding an old value -- and committing a report changes HEAD, so a HEAD-derived seed could never validate at the commit that carried the report. Co-authored-by: Isaac
MLflow refuses to clear a description, and the update sends the field unconditionally, so a config that drops it fails on a real workspace while the fake server accepted it. readplan-permissions changed the description instead of removing it, since removing it can no longer converge. Co-authored-by: Isaac
From the AWS comparison: a skip key naming a field inside a slice
("aliases[*].id") never matched the concrete "aliases[0].id" it expands to, so the
field was tested anyway and the backend rejected every value.
Keys are now matched the way the planner matches its own field rules, which also
makes naming a block skip everything beneath it -- so the ".*" suffix is gone.
Co-authored-by: Isaac
From the AWS comparison: an alias belongs to a model version and is managed through its own API, so UC does not echo aliases on a plain GET -- which is why the direct engine passes include_aliases=false and declares the field input_only. The fake server returned them anyway, so the remote appeared to hold what the config asked for and the field's real behaviour was hidden. registered_models now matches AWS row for row. Co-authored-by: Isaac
From the AWS comparison: an experiment name is an absolute workspace path and an artifact location needs a scheme, so the generic "x" was rejected outright. The UC trace location needs its catalog and schema together, like a job's git_source, so it is seeded rather than built up one field at a time. experiments matches AWS row for row. Co-authored-by: Isaac
…on values
From the AWS comparison. A retention period is validated in days (0, or 7 to 30), so
the generic 1 and 2 were rejected. The rest need workspace state this suite does not
provision: a Delta Sharing provider and share, a connection, an external location, a
customer-managed key.
options is skipped for a different reason worth noting: UpdateCatalog rejects the
field outright ("UpdateCatalog options can not be provided"), and the engine sends it
on every update whenever the config declares it -- so seeding it made every other
field of the catalog fail its update too.
catalogs matches AWS row for row.
Co-authored-by: Isaac
From the AWS comparison: external_locations cannot deploy against a real workspace at all -- it needs a storage credential with cloud IAM behind it -- so every field reported one BASE_ERROR and the golden diverged for a reason that says nothing about the engine. The invariant suite already excludes the same config from its cloud run. A type now declares "local_only: <reason>" in its value library and is skipped on cloud; its local golden stands, since a cloud run neither confirms nor contradicts it. Co-authored-by: Isaac
Two fixes from the AWS comparison, both about the suite rather than the engine.
An alert or dashboard is created inside ${workspace.resource_path} and the backend
404s on a missing parent, which is why a real deploy runs deploy.ResourcePathMkdir
first. This suite plans and applies directly, so it now runs that step too --
otherwise the whole type reported one BASE_ERROR that said nothing about any field.
A seeded value naming tester@databricks.com only exists on the fake server, so the
base fragment is now expanded with the same $VARS the corpus configs use.
Also: a base error's evidence went through firstError, which truncates to 140
characters -- so the full report, whose whole purpose is to carry the backend's own
words, showed a cut-off message.
Co-authored-by: Isaac
…brary From the AWS comparison. An enum field was getting the generic "x" and "y", which a real backend rejects or silently ignores, so the field reported nothing about whether the engine handles a change to it -- the alerts report was full of those. Every generated SDK enum declares its own values through a Values() method, so they are read from there instead, which fixes enum fields across every resource type. The alerts library also skips what the suite cannot provision: a second warehouse or workspace folder (and the backend refuses to move an alert between folders at all), and a registered notification destination. A subscription and a run_as now name the workspace's own user. Co-authored-by: Isaac
…texts Both from the adversarial review. containerValues recorded a slice or map with its elements shallow-copied, so the pointers inside an element were shared with the live resource: editing a field under clusters.init_scripts[0].workspace then reached into the value saved for the init_scripts container's own transitions, and that container was tested against corrupted data. A plan the caller never applies was dropping its pendingApply on the floor, leaking the operation context and its ten-minute timer -- thousands of them on a cloud run. Such a caller now goes through readPlan, and the two paths that plan and then return early cancel explicitly. Co-authored-by: Isaac
A row read "cannot update resources.postgres_projects.foo: updating id=projects/test-pg-project-[UNIQU ...ovided in request", which is the question rather than the answer: the reason was "Field 'spec.budget_policy_id' is in update_mask but not provided in request", and the cut fell in the middle of it. The engine's own framing was the thing crowding it out, and it says only what the row already says -- the resource is the report's own filename, the operation follows from the transition. Dropping that prefix and widening the cap to 200 characters leaves every message in the catalog intact: 49 elided rows become 0, and the longest surviving detail is 213 characters. Worth the change beyond legibility. With the message whole, one row now reads "Field 'spec.default_endpoint_settings.suspension' is in update_mask but not provided" against a transition of autoscaling_limit_min_cu -- the engine naming a different field than the one being changed, which the elision had been hiding. Also restores the rune-safe truncation from an earlier commit, lost in a merge resolution: the ellipsis substituted for a URL is multi-byte, so a byte index can split it. Co-authored-by: Isaac
A field the API will not clear leaves the resource drifted, and the suite replaced the resource for it. That was the wrong response twice over. It was wasteful: on apps it cost a fresh app per non-converging field, 31 in one run, on a shared workspace that allows 100 in total. The sweep kept hitting "reached the maximum limit of 100 apps" for no reason but this. And it was wrong. The drift was real, and destroying the resource that carried it meant the next resource had none -- so the drift reappeared later and was attributed to whichever field was under test at the time. database_instances shows it plainly: COLLATERAL_DRIFT 46 rows to 0, OK 2 rows to 26, and capacity now reads UPDATE_IGNORED, which is what the API actually does with it, rather than POST_DEPLOY_DRIFT. A resource is replaced when it will no longer deploy at all -- which is what the original comment was really guarding against, and is rare. Drift on its own goes into the baseline, where remeasure already knew how to put it, and every later field is measured against the drift that is really there. Across the catalog: COLLATERAL_DRIFT 46 to 20. Co-authored-by: Isaac
…n assume it
A failed deploy is not evidence that a resource is unusable -- the API may simply refuse to put one
field back. So ask it something it must be able to answer.
Three outcomes now, and only the last spends a new resource:
- Base comes back. Any remaining drift is real and goes into the baseline.
- Base does not, but the state the transitions left does. The API had already accepted that state, so
a deploy of it succeeds unless the resource itself is broken -- it answered, so the resource is
alive and updatable and only this one field is stuck. Base advances to what is reachable, so later
fields start from a state the API accepts instead of re-failing on this field all run.
- Neither deploys: past repair, replace it.
The probe is a snapshot rather than a reconstruction -- the config as the field's transitions left it is
already the state the remote holds -- so the whole thing is a switch with three arms.
Verdicts improve again, because a discarded resource was taking its drift with it and the next field was
being blamed for it: OK 1486 to 1488, and postgres_projects' BASE_ERROR 14 to 12 with the remaining ones
naming autoscaling_limit_min_cu, the field actually under test, instead of suspension.
Co-authored-by: Isaac
… does "use_ml_runtime is not allowed with unspecified kind" -- the same constraint already recorded for is_single_node, on a fixture whose cluster has no kind. Found by the cloud run; the mock server stores the field either way. Co-authored-by: Isaac
Review found the three-arm switch was doing more than it needed. The first arm deployed, then called converged(), which deployed again -- and converged() answers false for both "the deploy failed" and "the plan is dirty", so a second failure was being recorded as drift. The arms also differed in whether they refreshed the baseline, so a clean restore left stale entries behind, and driftDetail would have suppressed a later drift on one of those paths. It is now one condition and one shared conclusion: try base, fall back to the state the transitions left, rebuild only if neither deploys -- then re-measure the baseline from whatever landed. Always replacing the baseline is what fixes the stale entries, and it is also simpler than deciding when not to. converged() and hasDrift() have no callers left, which is the clearest evidence the shape was wrong: their whole job was the second deploy this no longer does. No golden moves. Co-authored-by: Isaac
A rejected request is a statement about the request, not the resource: a 400 means the backend did not act, so the remote still holds what it had and the run carries on with it. What the rejection tells us depends on direction, and removing a value is the informative one -- a field the API will not let go of is required in practice, whatever the bundle schema says. Two verdicts, split by whether the schema already knew: CANNOT_DELETE_REQUIRED -- the schema marks it required, so the refusal is the contract working. CANNOT_DELETE_NOT_REQUIRED -- it does not, so one of the two is wrong and is worth a look. Reaching the second needs absent tested on required fields, which the suite used to skip on the grounds that a config missing one does not validate. That was the wrong authority: validate.Required only warns, and the edit happens after it runs, so the API was always the one answering. Now it answers for every field. 8 rows come back CANNOT_DELETE_REQUIRED and 47 CANNOT_DELETE_NOT_REQUIRED -- cluster policies' name and definition, a model's description, postgres is_protected and no_expiry. The same classifier reads the other direction too: a field that cannot be cleared cannot be a starting value either, so those rows say so instead of carrying a bare error. BASE_ERROR drops from 93 to 65. One honest limit: CANNOT_DELETE_NOT_REQUIRED groups two causes. Most are the API requiring a field the schema does not mark, which is the finding it is named for; some are the engine naming a field in update_mask and then omitting it, which is a bug of ours and not a statement about requiredness at all. The API's own words are in the detail, which is what tells them apart. Co-authored-by: Isaac
CANNOT_DELETE_NOT_REQUIRED was carrying three unrelated facts, because any 400 on a removal was read as
the API insisting on the value. The messages tell them apart, so the classifier now reads them -- with
the phrase list taken from real responses rather than guessed, and written down next to the service that
produced each one:
UPDATE_MASK_OMITTED the engine named the field in update_mask and left it out of the body.
Our defect, not a fact about the field, and checked first because it
would otherwise read as the API demanding a value it was never sent.
Checked before the direction gate too: a malformed mask is malformed
whichever way the field was moving, which is what turned up 33 more rows
that had been sitting in BACKEND_ERROR.
CANNOT_DELETE_{NOT_,}REQUIRED the message says the value is needed -- must be supplied, cannot be
empty, Missing required field. Split by whether the schema agrees.
BACKEND_ERROR everything else, which now means what it says: an enum value the API
does not accept, a name that already exists, a parent that is missing.
Of the 47 rows previously grouped as one: 31 are the update_mask defect, 12 are the API requiring a
field the schema does not mark, and 4 are rows where both agree. The two "Unknown library" rows went
back to being plain backend errors, which is correct -- that message says nothing about requiredness.
error_code does the coarse split it can be trusted for: INVALID_PARAMETER_VALUE and BAD_REQUEST mean the
request was refused. Every message above shares those codes, so the fine split has to come from the
text, and the phrase list is the honest version of that.
Co-authored-by: Isaac
Both from the cursor review, both the same shape -- an operation that failed left something behind for the next field to trip over. A rejected edit is now undone. setNode allocates the containers on the way to a leaf before it can find out the leaf will not take the value, so an UNSETTABLE transition left a half-built block in the config while the caller moved on believing nothing had happened. setField snapshots and restores, which covers both of its call sites and any future one. Latent today -- every recorded UNSETTABLE fails before mutating anything -- so this is a guard, not a fix for an observed wrong row. A base the run could not reset is now recorded. When the API refuses to put a field back, the run keeps the state the API took, because throwing the resource away costs an app on a shared workspace and blames the discarded drift on later fields. The baseline keeps those later verdicts honest, but the config they deploy still carries the field, so it rides along in their update masks -- and nothing said so. A BASE_SHIFTED row now names the field, which puts the caveat in the golden where a reader comparing two types' reports will see it. Three rows: apps' description, pipelines' budget_policy_id, and postgres_endpoints' autoscaling_limit_min_cu -- the same fields the CANNOT_DELETE and UPDATE_MASK_OMITTED rows already name, which is the consistency check on it. Co-authored-by: Isaac
The cloud sweep's first divergence, and exactly the kind this suite exists to find: a real workspace refuses to clear display_name, query_text, or either half of a schedule -- "Field 'display_name' is required, expected non-default value (not "")!" -- on update as much as on create, while the fake server took all four. Sixteen alert rows read OK locally and CANNOT_DELETE_REQUIRED on aws. The fake server now validates them, so both sides agree on CANNOT_DELETE_REQUIRED. A schedule stays optional; an alert that has one has to fill both of its fields, there being no such thing as a schedule without a cron expression or a timezone. Message shape copied from the postgres handler, which already models this backend's wording. Also widens the summary's verdict column from 18 to 26, the length of the longest verdict name -- the counts had stopped lining up as soon as one of the new names appeared. Co-authored-by: Isaac
From the serial cloud sweep, each found by aws and the fake server disagreeing: experiments: the backend refuses a create or update with no name -- "An experiment name must be provided" -- while the fake server's update silently skipped an empty NewName, which is the UPDATE_IGNORED those rows used to read. Now refused on both sides, and the four rows say CANNOT_DELETE_REQUIRED. That message also exposed a gap in the classifier's phrase list: it knew "must be supplied" but not "must be provided", so the rows landed in BACKEND_ERROR. Added, with the service that produced it noted beside the rest. clusters: dependency_mode needs DBR 19 and the fixture runs the default spark version, which is DBR 13. Skipped rather than raising the runtime for one field, which would change what every other field of the type is measured on. The fake server has no DBR notion, so all six rows read OK locally. genie_spaces: title is omitempty in the update request, so clearing it never reaches the backend. Every transition the suite records as starting from absent actually starts from the previous value, which makes the from-labels fiction and the verdicts order-dependent -- aws and the fake server disagree on four of six rows without either being wrong. Skipped, because a field that cannot be cleared has no starting value the suite can establish. I also had the fake server drop title updates to match what aws showed, then reverted it: aws applied one of the six, so "never applied" is not something I can claim. Co-authored-by: Isaac
The cloud sweep's remaining eight divergences are all one fake-server gap: it accepted an empty name (or
other identity value) where the API refuses one, so clearing such a field read OK locally and failed on
aws. Each message below is the backend's own, taken from the sweep's logs rather than guessed:
jobs environment key charset, git_source.git_url, a nameless job parameter, and a
periodic trigger interval below 1. Extends the existing git-provider check into one
validateJobSettings covering the settings a job is created or reset with.
pipelines a deployment block with no kind
schemas empty name or catalog_name, in Unity Catalog's "is not a valid name" wording
volumes same, plus volume_type, which CreateVolume requires
secrets same wording again, so it reuses the helper schemas introduced
serving endpts the 63-character alphanumeric name constraint
vector search the 50-character lowercase one (with the backend's doubled space, kept verbatim)
secret scopes "Scope name must be non-empty and at most 128 characters!"
The verdicts these produce stay BACKEND_ERROR and BASE_ERROR rather than CANNOT_DELETE_*, and that is
right: the API is complaining that the value it was sent is invalid, not that the field is required.
Reading requiredness into "must contain less than 50 characters" would be inference, not evidence. Where
the backend does say required -- CreateVolume's volume_type, pipelines' deployment.kind -- the rows say
CANNOT_DELETE_REQUIRED on their own.
Two corrections along the way. I first put the secret-name check in libs/testserver/secrets.go, which
serves scope-based secrets; these are Unity Catalog secrets on a different route, so that edit was
reverted and redone in uc_secrets.go. And a blanket string replace briefly dropped two of the four job
checks from the create path by rewriting the SubmitRun call site and JobsCreate identically.
Co-authored-by: Isaac
The testserver now refuses a volume without one, the way CreateVolume does, and this fixture builds a volume from a bare struct. Same shape as the sql_warehouses fixture fix earlier on this branch: teaching the fake server a required field breaks every test that was relying on it not being required. Mine to fix, and mine for not catching sooner: after adding that validation I re-ran the autotest package and the acceptance suite but not ./bundle/..., so this sat red locally until the rebase made me run everything. Co-authored-by: Isaac
…_version repl #6549 normalized plan_version in every recorded plan, including this test's golden, and its version replaced the one this branch had regenerated. The golden then expected an email_notifications block the branch's testserver no longer produces at create. Regenerated, so it carries both: main's [PLAN_VERSION] repl and this branch's create behaviour. Co-authored-by: Isaac
The SDK bump (#6448) swapped BranchSpec's ttl (a *duration.Duration the catalog cannot move a generic value through, so it was never covered) for source_snapshot (a string). The catalog walks the new field automatically -- six OK_RECREATE transitions, a branch being recreated when it changes -- which is the coverage a new settable field is supposed to get. No other type's golden moved. Co-authored-by: Isaac
#6084 refactored the immutable folder upload into a direct-engine resource, adding internal_immutable_snapshots to SupportedResources. The drivenTypes guard demanded a fixture for it -- which is the guard working, a new supported type must be covered -- but this one is marked bundle:"internal" on config.Resources: an engine construct with no user-authored config, the same category as permissions and grants. So it is excluded the same way, by the bundle:"internal" tag rather than a name-prefix guess, reading it off config.Resources with structwalk the way resources_test.go does. Co-authored-by: Isaac
…edules Two things from the latest main: dstate.Open gained WithDeploymentHistory and OpenDmsArgs (#6094). The harness passes WithDeploymentHistory(false) and a zero OpenDmsArgs{} at both call sites -- this suite drives a local state file, not a recorded DMS deployment, the same as bind.go's callers. The PyDABs codegen generated postgres_snapshot_schedules, and drivenTypes demanded a fixture for it -- the new-resource guard working. Added one, local_only like the rest of the postgres family (the project holds its name until a soft delete is purged, so a recreate cannot rebuild the chain). The schedule attaches to the project's implicitly-created "production" root branch; the backend refuses any other ("not allowed to snapshot non-root branch"), so the fixture follows acceptance/bundle/resources/postgres_snapshot_schedules/basic rather than creating its own branch. Co-authored-by: Isaac
…lan fix #6589 made the direct engine match recreate rules bidirectionally, so a change to a nested field inside a block covered by a recreate rule is now caught. Seven ingestion_definition.* transitions on pipelines move OK -> OK_RECREATE as a result -- connector_type, the data_staging_options catalog/schema, and several table_configuration leaves. Both are passing verdicts; this records the improved classification. Co-authored-by: Isaac
The cloud sweep's first divergence. A real workspace refuses to clear deployment.kind ("Missing required
field: new_settings.deployment.kind") or trigger.periodic.unit; the fake server accepted both, so eight
job rows read OK locally and CANNOT_DELETE_REQUIRED on aws. DABs stamps deployment.kind=BUNDLE on every
job it deploys, which is why the field is effectively always present and clearing it is refused -- so
even the absent<->value rows fail, at the step that tries to reach absent.
validateJobSettings now rejects a deployment block with no kind and a periodic trigger with no unit,
with the backend's own message and the field-path prefix it uses (empty on create, new_settings. on
reset). Both sides now agree on the eight CANNOT_DELETE_REQUIRED rows.
Co-authored-by: Isaac
The cloud sweep's second divergence. A real workspace refuses a tag with an empty key ("Endpoint tag
keys must be defined."), so clearing tags[0].key fails; the fake server stored the keyless tag, and four
rows read OK locally and BASE_ERROR/BACKEND_ERROR on aws. PatchTags now rejects an empty AddTags key with
the backend's message -- the engine wraps it as "failed to update tags: ...". Both sides now agree.
Co-authored-by: Isaac
The cloud sweep's third divergence. endpoint_type is required on create ("endpoint_type is required."),
so clearing it is refused; the fake server stored a typeless endpoint, and four rows read OK_RECREATE
locally and CANNOT_DELETE_REQUIRED on aws. Create now rejects an empty endpoint_type with the backend's
message. Both sides now agree.
Co-authored-by: Isaac
The cloud sweep's fourth divergence, and the narrowest: the message matched but the fake server returned the 400 with no error_code, so the row read (400 <nil>) locally against (400 INVALID_PARAMETER_VALUE) on aws. Added the code to the existing response. Both sides now agree. Co-authored-by: Isaac
The cloud sweep's fifth divergence, and the largest (160 rows). An ingestion pipeline must name exactly
one source -- connection_name, ingestion_gateway_id, or ingest_from_uc_foreign_catalog -- and a real
workspace refuses one with none ("Found unsupported source option ... unsupported source option:
Empty"). The base seeds no ingestion_definition, so every leaf the catalog set built a sourceless block:
the mock server stored it (OK/OK_RECREATE) while aws rejected it (BASE_ERROR/BACKEND_ERROR).
All three source options need UC infrastructure the suite does not provision, so no leaf under
ingestion_definition is deployable on cloud. Skipped the whole subtree with ingestion_definition.*,
subsuming the three source-field skips that were already there for the same underlying reason. Covering
it needs a fixture that provisions a real ingestion source.
Co-authored-by: Isaac
The cloud sweep's sixth divergence. A uc_trace_location, once present, has to name both its catalog and
schema; a real workspace refuses either cleared ("trace_location.uc_trace_location.catalog is
required."). trace_location is set only at create, so the engine recreates to change it -- which is why
these read OK_RECREATE locally, where the fake server accepted the sourceless block, and
CANNOT_DELETE_REQUIRED on aws, where the recreate's create fails. ExperimentCreate now enforces both.
Co-authored-by: Isaac
The cloud sweep's seventh divergence, in two parts, both in AppsCreateUpdate (the POST /update path the engine uses). Clearing a masked field was ignored. The handler only set fields present in the update body and never cleared one absent from it, and it unmarshalled the merged map back into the old struct -- where json.Unmarshal leaves an absent field untouched. So clearing description or user_api_scopes kept the old value (UPDATE_IGNORED locally) where aws applies it (OK). The merge now deletes a masked field absent from the body and unmarshals into a zero value, so a cleared field really clears. The map is built from the raw body, since re-marshalling the SDK struct would drop an explicitly-empty field. That then exposed the second part: compute_size is a backend default (MEDIUM), re-stamped by the platform after any update, but only AppsUpsert applied it, not AppsCreateUpdate. With clears now honoured, compute_size went empty on update and the engine's backend_default/remote_already_set suppression stopped firing -- OK locally where aws is SUPPRESSED. AppsCreateUpdate now re-applies the same defaults after the merge. apps matches aws row for row. Co-authored-by: Isaac
The latest main generated the model_services resource, and drivenTypes demanded a fixture -- the new-resource guard working. Added one modelled on acceptance/bundle/invariant/configs/model_service.yml.tmpl: parent references the pre-existing main.default schema, model_service_id is the other half of the immutable id, comment is the settable scalar. The parent id-field rows show the standard invalid-name BASE_ERROR a generic value produces, the same as other name-composed id fields. Co-authored-by: Isaac
Cloud verification of the new model_services resource surfaced two fidelity gaps the fake server had.
A model service must be created with a config block -- the backend refuses one without ("config is
required") -- so base seeds a routing config to a pay-per-token system foundation model
(models/system.ai.databricks-gpt-5), which needs no workspace infrastructure. And its destinations
cannot be emptied once set: the backend refuses it and points at DeleteModelService instead. The fake
server accepted both, so the config subtree read OK locally where aws rejects it. Create now requires
config, and update refuses a change that leaves no destination, both with the backend's own messages.
The cloud run that surfaced this was also noisy with 499 CANCELLED from a degraded workspace (clusters
could not provision at the time either); the destinations rows that came back cleanly match this golden,
and model_services wants a clean cloud re-verification once the workspace is healthy.
Co-authored-by: Isaac
…sing Refines the previous commit. Four acceptance tests create a model service with no config block, so requiring config on create broke them -- reverted, the catalog fixture carries its own config and does not need the fake server to enforce that. And the destinations check is now conditional: it fires only when an update would empty destinations a service already had, leaving a service that never had a config block untouched. The catalog golden is unchanged (its base seeds destinations, so clearing them is still refused), and the model_services + bind acceptance tests pass. Co-authored-by: Isaac
Both are my own fidelity fixes whose goldens the rebase left behind, not behaviour changes here: - models/readplan-permissions: 03448f62c made the testserver reject an empty MLflow description and changed the script to modify the description rather than remove it, but the output.txt kept the old "and description" title. Regenerated to "change description". - model_serving_endpoints/update/email-notifications: 9d55fe09d made a serving endpoint's create not echo email_notifications (matching aws), so the read right after deploy is null, not the seeded value. Regenerated. Co-authored-by: Isaac
…the generator change #6618 expands reused nested types per-path in the direct resources generator, which changes the field paths the catalog enumerates for these two types. Both are local_only, so this is their local truth -- verdict counts shift (database_instances' COLLATERAL_DRIFT findings actually decrease, external_locations stays finding-free), no new problems, deterministic. Regenerated to match the new structure. Co-authored-by: Isaac
denik
force-pushed
the
denik/extensive-test
branch
from
September 11, 2026 19:59
d34b72b to
0528be3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
bundle/direct/autotest: a per-field catalog of how the direct engine handles every field a usercan set. For each field it walks every ordered pair of a small value set —
absentincluded, so addingand removing a field are just the pairs with
absenton one side — and records three things per move:did the plan propose a change, did the apply succeed, did the next plan converge.
The outcome is a verdict word in a committed golden. A bad outcome is recorded, not failed, so the
report is a map of the engine's behaviour rather than a wall of failures; what fails the test is a
verdict changing. All 34 resource types the engine supports, ~3900 observations.
Both a local run against
libs/testserverand a real-workspace run are held to the same golden, so adivergence means the fake server does not match the API. That is where most of this PR's findings come
from — 14 fake-server fidelity fixes, each carrying the API's own error text:
cluster_size, andmax_num_clustersin 1..40, and thebackend defaults
enable_photonandspot_instance_policycreated with neither gets storage, and the later update is then refused
email_notifications; an update doesautotermination_minuteshas a 10-minute minimumtarget_qpson a vector search endpoint is omitempty, so the API drops a clearThree of those exposed acceptance fixtures asserting behaviour the API does not have. All were
Cloud = false, so nothing had ever checked them:bind/pipelines/updatebound a storage-defaultedpipeline to a config asking for a catalog;
create-scopesent the bundle schema's field name to a rawAPI call, so the CLI dropped the metadata and the whole script aborted once the fake required it; and
the serving-endpoint fix sharpened a test whose own comment describes a TF bug the fake had been hiding
half of.
Engine findings the goldens record: 27
postgres_projectsrows name a field inupdate_maskand thenomit it from the body (#6441 and #6443 fixed 13 of these while this branch was open);
pipelines.catalogis absent fromrecreate_on_changes, so the engine sends an update the API refuses.Running it
./task testcovers the local run. Two new tasks for the real-workspace runs:./task autotest-cloud— every field; nightly./task autotest-cloud-pr—-sample 2, for PRs: two fields per type, picked from HEAD sosuccessive commits cover different ground while one run's picks follow from its SHA
output/holds the 34 committed goldens and nothing else;logs/holds the full report per type andis gitignored wholesale.
State of the cloud verification
In progress. 14 of the 34 types declare
local_onlywith a reason — an external location needsIAM-backed storage, an instance pool cannot be deleted again, a postgres project holds its name after
deletion, a job run would start a real job per transition. Of the 20 that can be driven, verification
against the current goldens is under way; each one that diverged so far has been a fixture value the
fake accepts and the API refuses, or a fake-server gap, and is fixed in this branch.
Test-only; no changelog fragment.
This pull request and its description were written by Isaac.