Skip to content

kola: Restore kola.json metadata fallback - #4608

Open
Amir-A664 wants to merge 1 commit into
coreos:mainfrom
Amir-A664:fix/kola-external-test-metadata
Open

kola: Restore kola.json metadata fallback#4608
Amir-A664 wants to merge 1 commit into
coreos:mainfrom
Amir-A664:fix/kola-external-test-metadata

Conversation

@Amir-A664

@Amir-A664 Amir-A664 commented Aug 20, 2026

Copy link
Copy Markdown

Closes #4392

Summary

  • leave external-test metadata unset when an executable has no inline JSON or YAML block, allowing registerExternalTest to fall back to the directory's kola.json
  • preserve the existing Exclusive: true default when inline metadata is present
  • add regression coverage for absent metadata, JSON and YAML metadata, and explicit non-exclusive settings

Testing

  • go test -mod=vendor ./mantle/kola
  • go test -mod=vendor ./mantle/kola -run TestMetadataFromTestBinary -count=1 -v
  • go vet -mod=vendor ./mantle/...
  • gofmt -d mantle/kola/harness.go mantle/kola/harness_test.go

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of optional metadata embedded in test scripts.
    • Correctly parses both JSON and YAML metadata, including explicit exclusive: false values.
    • Scripts without embedded metadata are now handled appropriately.
  • Tests

    • Added coverage for JSON and YAML metadata, default values, and scripts without metadata.

metadataFromTestBinary initialized a non-nil metadata value before
checking for inline metadata. As a result, registerExternalTest could
never fall back to the directory's kola.json.

Allocate metadata only after detecting JSON or YAML. Add regression
coverage for missing inline metadata, both formats, and exclusivity
defaults.

Closes: coreos#4392
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cf95b21-f6ae-4f6e-b2bb-56f69e8ec8d5

📥 Commits

Reviewing files that changed from the base of the PR and between 4524392 and 37eaa71.

📒 Files selected for processing (2)
  • mantle/kola/harness.go
  • mantle/kola/harness_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The external test metadata parser now returns nil when no inline metadata exists. JSON and YAML metadata retain exclusive defaults, while explicit exclusive: false remains supported. New table-driven tests cover these parsing cases.

Changes

External test metadata

Layer / File(s) Summary
Metadata parser and validation
mantle/kola/harness.go, mantle/kola/harness_test.go
The parser creates metadata only after detecting JSON or YAML metadata. YAML parsing initializes Exclusive: true directly. Tests cover absent metadata, default exclusivity, and explicit exclusive: false for both formats.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 37eaa

The PR restores metadata fallback behavior and adds regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: dustymabe

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the restoration of the kola.json metadata fallback, which is the primary change.
Linked Issues check ✅ Passed The changes satisfy issue #4392 by allowing absent inline metadata to fall back to directory kola.json while preserving inline metadata precedence.
Out of Scope Changes check ✅ Passed The implementation and regression tests are limited to external-test metadata fallback and inline metadata behavior described in issue #4392.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Aug 20, 2026

Copy link
Copy Markdown

Hi @Amir-A664. Thanks for your PR.

I'm waiting for a coreos member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@Amir-A664
Amir-A664 marked this pull request as ready for review August 20, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kola.json is not used on external tests

1 participant