Skip to content

Remove stale dev/diffs test exclusions whose tracking issues are closed #5569

Closed

Description

@andygrove

Three exclusions in dev/diffs/*.diff cite Comet issues that have since been fixed, so they now suppress Spark SQL test coverage for no reason. Each is present in more than one version diff, so they should be removed across all affected diffs in one pass rather than in whichever version PR happens to touch them next: new version diffs are seeded from the previous version's, so a partial fix diverges the diffs and the next reseed reintroduces the stale entry.

Surfaced in review of #4950 by @sunchao.

1. Variant shredding suites

VariantShreddingSuite and ParquetVariantShreddingSuite are given an IgnoreCometSuite mixin, which skips all 15 tests when ENABLE_COMET=true.

This is the highest-value one: it drops regression coverage for the silent-null corruption that #4084 fixed.

2. having-and-order-by-recursive-type-name-resolution.sql

The input file carries a --SET spark.comet.enabled = false header, which runs all 45 queries without Comet.

3. Parquet V2 writer matrix in ParquetTypeWideningSuite

withAllParquetWriters { ... } is commented out, leaving all 82 generated widening cases on Parquet's default V1 writer.

  • Cited issue: Support DELTA_BINARY_PACKED and DELTA_BYTE_ARRAY #574, closed: the active native_datafusion and native_iceberg_compat scans support DELTA_BINARY_PACKED, DELTA_BYTE_ARRAY and DELTA_LENGTH_BYTE_ARRAY.
  • The related SPARK-40128 read DELTA_LENGTH_BYTE_ARRAY encoded strings test in ParquetIOSuite is still tagged IgnoreComet("Comet doesn't support DELTA encoding yet") and should be re-enabled with it.
  • Affected: dev/diffs/4.0.4.diff, dev/diffs/4.1.3.diff, dev/diffs/4.2.0.diff (byte-identical in all three).
  • If the legacy native_comet scan still cannot read these encodings, scope the exclusion to that scan implementation rather than removing the whole matrix.

Suggested approach

One PR per item, each removing the exclusion from every diff that carries it, verified by the corresponding Spark SQL Tests CI job for each affected version. Where a specific case still fails, replace the blanket exclusion with a case-scoped IgnoreComet linked to a current issue, per the rule in docs/source/contributor-guide/adding_a_new_spark_version.md ("do not disable whole suites").

Activity

  1. added
    enhancementNew feature or request
    testTesting related
    priority:mediumFunctional bugs, performance regressions, broken features
    area:ciCI/CD, GitHub Actions, build tooling
    on Aug 31, 2026
  2. rich7420 commented on Sep 7, 2026

    @rich7420
    Contributor

    take

  3. andygrove commented on Sep 22, 2026

    @andygrove
    MemberAuthor

    All three items are now fixed, though not in the one-pass-per-item shape this issue proposed.

    Item 4.0.4 4.1.3 4.2.0
    1. Variant shredding suites already clean #5745 #4950
    2. having-and-order-by-recursive-type-name-resolution.sql n/a #5755 #4950
    3. Parquet V2 writer matrix + DELTA_LENGTH_BYTE_ARRAY #5760 #5760 #4950

    The 4.0 and 4.1 halves landed on main between 2026-09-08 and 2026-09-09. The 4.2 half is in #4950 and is not yet merged, so this issue should close with that PR rather than now.

    One thing this issue predicted and that turned out to be worth recording: the reseed hazard is real, but it does not need a reseed to bite. dev/diffs/4.2.0.diff exists only on the #4950 branch, so none of #5745, #5755 or #5760 reached it, and merging main into that branch produced no conflict at any point -- the 4.2 diff simply sat at its 2026-08-31 state while the sibling diffs moved on. By the time it was checked, seven upstream commits to 4.1.3.diff had accumulated. The general fix is to not leave a supported version's suite outside the automated tiers: #4950 now also moves Spark 4.2 into the nightly tier, since a diff is kept current as a side effect of its suite running.

  4. dwsmith1983 commented on Oct 4, 2026

    @dwsmith1983
    Contributor

    #4950 has merged now, so all three exclusions are gone from every version diff: #5745, #5755 and #5760 cleaned up 4.0 and 4.1, and #4950 added 4.2.0.diff without them. The only Variant exclusion left is one test in ParquetVariantShreddingSuite, ignored for #5741, which is a separate issue. I think this can be closed.

  5. rich7420 commented on Oct 16, 2026

    @rich7420
    Contributor

    @andygrove hi , could you help me to close this?

  6. andygrove commented on Oct 16, 2026

    @andygrove
    MemberAuthor

    Thank you @dwsmith1983 and @rich7420

  7. rich7420 commented on Oct 16, 2026

    @rich7420
    Contributor

    thanks for your help @andygrove !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:ciCI/CD, GitHub Actions, build toolingenhancementNew feature or requestpriority:mediumFunctional bugs, performance regressions, broken featurestestTesting related

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions