test(delta-harness): add standard matrix - #682
Conversation
Adds a self-contained Scala behavioral test harness that characterizes OpenHouse + Apache Iceberg table behavior end-to-end. The harness crosses a large matrix of table layouts (partitioning, MoR/CoW, ordered writes, nested types) with DDL, DML, maintenance, branching/WAP, streaming, and negative-path operations, asserting deltas against observed pre-state so each case holds under any layout. It runs locally against a real embedded OpenHouse catalog (harness/openhouse/Env.scala boots OpenHouseLocalServer + the OpenHouse Spark catalog; see run-openhouse.sh and HARNESS-GUIDE.md). The scenario and framework sources are also structured as a publishable Gradle library module (openhouse-spark-delta-harness_2.12) that excludes the embedded-only Env so downstream environments can supply their own adapter. Genuine product or upstream bugs are tagged in Plan.knownBugs with a prose explanation and skipped rather than silently passed, so the suite stays green while documenting the defect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds TESTING-MATRIX.md, a living reference that explains the harness as a cross product of independent axes (operation family, data file format, partitioning, write mode, schema, preparation lineage, and reference routing). Documents how a case id reads, the CoreTable/NestedTypesTable/TypesTable schemas, the table layouts, the preparation lineages, and each operation family including the DDL sub-families. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move each test's preparation, action, and assertions into its scenario file so the complete behavior is readable in one place. Keep reusable preparation recipes while creating a fresh table for every case. Preserve the exact 2,574-case catalog, ordering, and known-bug behavior with regression tests for the catalog fingerprint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explain the scenario-owned test structure, immutable preparations, and fresh-table isolation used by the localized test cases. Describe the matrix as living documentation for the current harness architecture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the harness guide and testing matrix from the implementation PR so the documentation can be reviewed in a separate stacked change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| TablePreparation( | ||
| layout.label, | ||
| createAndSeed(layout, 3) | ||
| .sql("ddl")(table => s"ALTER TABLE $table ADD COLUMN cc int")(), |
There was a problem hiding this comment.
what ddl does is unclear here. ITs a function.
| layout.label.endsWith("/orc")) | ||
| .flatMap { layout => | ||
| val preparations = List( | ||
| TablePreparation( |
There was a problem hiding this comment.
Each preparation needs a description.
| layout.label.endsWith("/parquet") || | ||
| layout.label.endsWith("/orc")) |
There was a problem hiding this comment.
Why not just ahve a list of layouts?
| "ddlConsume:writeOrder."), | ||
| TablePreparation( | ||
| layout.label, | ||
| createAndSeed(layout, 3) |
There was a problem hiding this comment.
Create and seed should be separate calls. What is seeded should be visible or standard.
Separate table preparations from DML operations so each case shows its starting state, mutation, and relative assertions in one place. Keep feature-owned scenarios in removable RTAS, merge-on-read, and branch layers while preserving the exact ordered 2,572-case catalog. Run the same published sources through the local Gradle task and the acceptance adapter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the standard branch focused on copy-on-write behavior, shared table preparations, bespoke DDL coverage, and the local execution framework. Remove RTAS, merge-on-read, branch, and WAP scenario ownership from this layer. Pin the resulting ordered standard catalog at 1,181 cases so each child branch can add one reviewable feature delta. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| // (b) toJson takes no format-version argument, so the key serializes the same regardless of format version. | ||
| // (c) The value round-trips through fromJson then toJson. | ||
| val reparsed = org.apache.iceberg.SchemaParser.fromJson(json) | ||
| val json2 = org.apache.iceberg.SchemaParser.toJson(reparsed) |
There was a problem hiding this comment.
Just import. Apply this across all of these deep calls.
Keep runtime case metadata limited to stable identifiers and execution state. Put preparation and test explanations beside their Scala behavior so reviewers can read each case without tracing string registries. Generate a fresh UUID for every table and begin cleanup only after the preparation creates it, which preserves any pre-existing table on a name conflict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract the owned-table cleanup state machine behind a package-private boundary so its failure paths can be tested without starting Spark. Pin conflict preservation, successful cleanup, and suppression of cleanup failure behind the primary test failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin the remaining ownership outcome: when the test body succeeds and cleanup fails, the cleanup failure must surface to the runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| deleteByNullCondition) | ||
|
|
||
| /** | ||
| * DELETE WHERE datepartition = '2024-01-01-00' removes the rows in that partition value, keeps |
There was a problem hiding this comment.
datepartition is a smell. Instead of that, its a date column that is partitioned. Adding the name partition to the col sows confusion.
| // size, and the compaction plan. These behaviors have no catalog SQL surface of their own, so a | ||
| // case reaches them through the Iceberg API or a Spark configuration and asserts the result a | ||
| // caller can observe. | ||
| trait ForkScenarios extends ScenarioKit { |
There was a problem hiding this comment.
forkscenarios are Iceberg tests. they should all run agains the fork, so its unclear why this is a file vs denormalized into other features.
| * Reflectively builds an optional int NestedField carrying the given initial default. Returns | ||
| * None when the builder API is absent, so a caller can assert that absence directly. | ||
| */ | ||
| private def buildDefaultedIntField(id: Int, name: String, dflt: Int): Option[org.apache.iceberg.types.Types.NestedField] = { |
There was a problem hiding this comment.
move default values to its own PR.
| * DataFrame into a 4-partition table under each mode therefore yields at least as many files | ||
| * under the default as under HASH. The file format is the parameter. | ||
| */ | ||
| private def forkPartitionDistDefault(fmt: String)(ctx: Ctx): Unit = { |
There was a problem hiding this comment.
this test doesn't seem useful. is it? a write distribution mode test makes sense but not a None test. To me this is purely spark and nothing specific to iceberg being tested. That may be fine depending on how we phrase it.
| * path that supplies one. Reflection reaches the builder and getProperties because some Iceberg | ||
| * artifacts leave them out of the public compiled API. | ||
| */ | ||
| private def forkFileReplicationFactor(ctx: Ctx): Unit = { |
There was a problem hiding this comment.
REplicaiton factor should be tested. its not a fork, but a feature.
| * plans one task group, and a split size below one file plans one group per file. The file format | ||
| * is the parameter. | ||
| */ | ||
| private def forkSplitSize(fmt: String)(ctx: Ctx): Unit = { |
There was a problem hiding this comment.
again, these are iceberg features not a "fork"
| import scala.reflect.{ClassTag, classTag} | ||
| import scala.util.control.NonFatal | ||
|
|
||
| // The copy-on-write reader, writer and hazard families. The reader and writer cases pin the |
There was a problem hiding this comment.
CoW vs MoR is the test. "hazard" makes no sense.
mkuchenbecker
left a comment
There was a problem hiding this comment.
I would reindex on capabilities vs Hazard / Fork / etc. More files is fine, ideal case is a new file + a delta to integrate it.
| val typesLayouts: List[Layout] = | ||
| List("parquet", "orc", "avro").map(format => Layout(s"types-unpartitioned/$format", table => |
There was a problem hiding this comment.
constructing this list literal is suspect vs getting a standard "AllFileTypes". this is a nit for now, but a standard list is perferred.
Reflow harness documentation to the repository's 120-column target and explain the DML operation and preparation matrix at its source. Name the reusable date column independently from partitioning so layouts, not column identifiers, express partition choices. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| object Main { | ||
| def main(args: Array[String]): Unit = { |
There was a problem hiding this comment.
Main seems to ahve nothing to do with env.
| } | ||
|
|
||
| /** The rejected DML statements, on the preparedCoreFormats preparations. */ | ||
| val negativeCases: List[Plan.Case] = |
There was a problem hiding this comment.
compositions seem like they should be at teh top or bottom (top is preferred IMO). We can organize by public and private.
Replace provenance and consequence buckets with capability-owned scenario files whose public contribution surfaces explain the catalog at a glance. Separate local runner code from the publishable harness, make preparations show creation and standard seeding explicitly, and reindex generic case IDs. Use generated table names and failure-preserving ownership boundaries for every case-owned table, view, registration, rename, and lock lifecycle. Move column-default coverage out of the standard layer for a dedicated follow-up PR while pinning the remaining 1,177-case catalog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -0,0 +1,219 @@ | |||
| package harness | |||
There was a problem hiding this comment.
This should fail when you attempt to do so across RTAS to a previous table. Make sure RTAS PR capturs that.
| @@ -0,0 +1,43 @@ | |||
| package harness | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Stack on top unless tags need RTAS testing.
| @@ -0,0 +1,137 @@ | |||
| package harness | |||
There was a problem hiding this comment.
can be stacked on top
| import java.nio.file.{Files, Paths} | ||
|
|
||
| /** | ||
| * Encryption: the OSS build writes table data in plaintext, because OpenHouse delegates table-data encryption to an |
There was a problem hiding this comment.
can be stacked on top unless there is the chance RTAS silently drops encryption.
|
|
||
| /** The plaintext data-file case, on the standard seeded Parquet table. */ | ||
| lazy val encryptionCases: List[Plan.Case] = | ||
| List(dataFilePlaintextCase(preparedStandardTable("parquet"))) |
There was a problem hiding this comment.
orc should be the default test choice or both orc and parquet. Both is better. I would jsut standardize to do both.
| @@ -0,0 +1,61 @@ | |||
| package harness | |||
There was a problem hiding this comment.
rtas should test that after an RTAS sort order can be changed or dropped if its not already
| @@ -0,0 +1,213 @@ | |||
| package harness | |||
There was a problem hiding this comment.
can be stacked on top
| @@ -0,0 +1,98 @@ | |||
| package harness | |||
There was a problem hiding this comment.
RTAS definately needs time travel testing as a negative test case.
| @@ -0,0 +1,161 @@ | |||
| package harness | |||
There was a problem hiding this comment.
can be stacked on top
| @@ -0,0 +1,50 @@ | |||
| package harness | |||
There was a problem hiding this comment.
can be stacked on top
Name every scenario source and trait ScenarioFoo so scenario files group together and the framework files remain visually distinct. Preserve the catalog contributions, IDs, ordering, count, and fingerprint unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| * here rather than derived from `Plan.contributions`, so adding, dropping, renaming or reordering a capability fails | ||
| * this test until the intended catalog shape is restated. | ||
| */ | ||
| private val expectedContributionNames = List( |
There was a problem hiding this comment.
this test is a tautaulogy.
| */ | ||
| final class CaseCatalogTest { | ||
| private val expectedCaseCount = 1177 | ||
| private val expectedCatalogSha256 = |
There was a problem hiding this comment.
does this regenerate automatically?
Keep PR 682 focused on reusable DDL and DML coverage while moving orthogonal capabilities to extension branches. - retain 642 Parquet and ORC foundation cases - extract reusable changelog and concurrency support - preserve Plan and Scenarios consumer compatibility - add extension-stable catalog and support contract tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
This PR adds the standard OpenHouse delta-harness foundation. Preparations document the table state they create, test definitions document the operation and observable result beside their assertions, and runtime output uses stable case IDs.
The standard catalog contains 1,181 cases. Replace Table As Select (RTAS), merge-on-read, branch, and write-audit-publish (WAP) coverage are isolated in stacked child PRs.
Stack
All OpenHouse PRs target
main. Review them in dependency order; each upper diff shrinks as its dependencies merge. The documentation PR depends on the standard framework, and the Airflow PR consumes the branch and WAP artifact.mainScope
Plan.Case,TablePreparation, andDmlTestCaselimited to stable IDs, executable behavior, and skip or known-bug metadata.Validation
The catalog fingerprint is the regression guard for case identity and order.
377f65959e3034c51e078fea72491444b06a6055f37c051184bdc379234b3d57.read.projectionpassed locally.ddl.renameTablepassed locally.Run a local slice with: