Skip to content

[SPARK-59327][BUILD] Add config test module - #58587

Open
nchammas wants to merge 2 commits into
apache:masterfrom
nchammas:config-test-module
Open

nchammas wants to merge 2 commits into
apache:masterfrom
nchammas:config-test-module

Conversation

@nchammas

@nchammas nchammas commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add a new config test module that runs the config/test sbt goal (which runs ConfigRegistrySuite).

Why are the changes needed?

When this new suite was added in #53488 I believe it was left out of the test modules by mistake.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Confirmed that ConfigRegistrySuite now runs on CI.

Was this patch authored or co-authored using generative AI tooling?

I wrote this patch with assistance from GitHub Copilot.

@nchammas nchammas changed the title [BUILD] Add config test module [SPARK-59327][BUILD] Add config test module Sep 8, 2026
@nchammas
nchammas marked this pull request as ready for review September 8, 2026 11:58
@nchammas
nchammas requested a review from cloud-fan September 8, 2026 12:00
@nchammas

nchammas commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

The CI failure seems unrelated.

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thank you @nchammas! Leaving up to folks with more build/CI/infra expertise to further review.

@nchammas

Copy link
Copy Markdown
Contributor Author

Tagging some others who previously reviewed #53488.

@cloud-fan cloud-fan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

The new config module matches the existing SBT project name and Maven reactor path, and its dependency edge to utils agrees with the build graph. The workflow matrix and precondition updates are internally consistent, so the existing ConfigRegistrySuite is now covered without changing user-facing Spark behavior. I found no actionable issues in the pinned patch.

Findings

0 total: 0 P0, 0 P1, 0 P2, 0 P3.

No findings.

@cloud-fan

Copy link
Copy Markdown
Contributor

I'll merge after CI is green

@nchammas

Copy link
Copy Markdown
Contributor Author

@cloud-fan - The CI failure is unrelated: #58587 (comment)

But I'll update from master anyway.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants