Skip to content

Fix root-safe provider test runtime stub [reduced-it] - #15985

Merged
res-life merged 1 commit into
NVIDIA:mainfrom
binmahone:fix-root-safe-provider-runtime-stub
Sep 14, 2026
Merged

res-life merged 1 commit into
NVIDIA:mainfrom
binmahone:fix-root-safe-provider-runtime-stub

Conversation

@binmahone

Copy link
Copy Markdown
Collaborator

Follow-up to #15944 and #15957.

Description

After both Iceberg packaging changes reached main, package-parallel-worlds.py began reading system_runtime_path from the loaded iceberg_runtime.py module. RootSafeProviderSelectionTest creates a minimal fake version of that module, but the fixture still defined only coordinates. As a result, the standard-assembler subtest failed with KeyError: 'system_runtime_path' before Maven compilation, causing install and verify matrix jobs on subsequent pull requests to fail or be canceled by fail-fast.

This change adds the missing system_runtime_path function to the fake module. It returns None, matching the test fixture's existing behavior when no explicit system Iceberg runtime is configured. Production code is unchanged.

Validation on current main (b23ab02e986767dfd26fde2e221d58658d9a2179):

$ python3 dist/scripts/tests/test_root_safe_provider_selection.py
.
----------------------------------------------------------------------
Ran 1 test in 0.251s

OK

The failure was also reproduced before the fixture change with the same KeyError reported by the Maven matrix in #15982.

This change was prepared with AI assistance and reviewed by the author.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Hongbin Ma <mahongbin@apache.org>
Copilot AI lite review requested due to automatic review settings September 14, 2026 06:58
@binmahone
binmahone requested a review from a team as a code owner September 14, 2026 06:58

Copilot AI 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.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge and correctly restores the test fixture’s compatibility with the assembler.

Summary

  • Adds the missing fake runtime-module interface.
  • Preserves the fixture’s intended no-system-runtime configuration.
  • Changes test code only; production packaging behavior is unchanged.

Reviews (1) · Last reviewed commit: "Fix root-safe provider test runtime stub"

@gerashegalov gerashegalov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@gerashegalov

Copy link
Copy Markdown
Collaborator

I was including it in #15958. But a dedicated fix is faster.

@gerashegalov

Copy link
Copy Markdown
Collaborator

build

@res-life
res-life merged commit d76eb37 into NVIDIA:main Sep 14, 2026
60 checks passed
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.

6 participants