Skip to content

test: add unit tests for LinkDevice scoring - #2411

Closed
aaka24 wants to merge 2 commits into
Project-HAMi:masterfrom
aaka24:add-metax-score-tests
Closed

test: add unit tests for LinkDevice scoring#2411
aaka24 wants to merge 2 commits into
Project-HAMi:masterfrom
aaka24:add-metax-score-tests

Conversation

@aaka24

@aaka24 aaka24 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds unit tests for the metax device link scoring logic in
pkg/device/metax/score.go.

The scoring functions (LinkDevice.score(), LinkDevices.Score(), and
LinkDevices.String()) previously had no dedicated test coverage.
This change adds table-driven tests covering normal cases and edge cases
without modifying any production code.

Changes (test-only, pkg/device/metax/score_test.go):

  • Added tests for LinkDevice.score():

    • Same UUID devices
    • Zero linkZone values
    • Matching linkZone values
    • Different linkZone values
  • Added tests for LinkDevices.Score():

    • Empty device list
    • Single device
    • Same-zone devices
    • Different-zone devices
    • Pairwise scoring across multiple devices
    • Mixed link zones
    • Zero link zones
  • Added tests for LinkDevices.String():

    • Empty device formatting
    • Non-empty device output validation

This improves coverage of metax topology scoring behavior and helps
prevent regressions in scheduler extender link scoring logic.

Which issue(s) this PR fixes:

Fixes #2410

Special notes for your reviewer:

CI failure appears unrelated to this PR. The failing package is pkg/scheduler, while this PR only adds unit tests for pkg/device/metax/score.go. The new metax tests pass locally with go test ./pkg/device/metax/....

Validation performed locally:

  • go test ./pkg/device/metax/... -run TestLinkDevice -v — pass
  • gofmt -l pkg/device/metax/score_test.go — clean
  • go vet ./pkg/device/metax/... — pass

This PR only adds unit tests and does not change production behavior.

The failure logs point to the scheduler package (pkg/scheduler), specifically around the test-pod-nodefail path. No scheduler files or production code are changed in this PR.

Does this PR introduce a user-facing change?:

No.

Summary by CodeRabbit

  • Tests
    • Added coverage for device link scoring across UUID and link-zone combinations.
    • Added tests for scoring device collections, including empty collections and zero link zones.
    • Added validation for readable device collection output, including brackets and UUIDs.

Signed-off-by: aaka24 <yaarsuperstar44@gmail.com>
@hami-robot hami-robot Bot added the kind/feature new function label Aug 6, 2026
@hami-robot
hami-robot Bot requested a review from chaunceyjiang August 6, 2026 14:15
@hami-robot
hami-robot Bot requested a review from mesutoezdil August 6, 2026 14:15
@hami-robot

hami-robot Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aaka24
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hami-robot hami-robot Bot added the size/L label Aug 6, 2026
@github-actions github-actions Bot removed the kind/feature new function label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Added table-driven unit tests for metax device scoring and collection string formatting. The tests cover UUIDs, link zones, pairwise score accumulation, empty collections, and device output.

Changes

Metax scoring tests

Layer / File(s) Summary
Device and collection scoring tests
pkg/device/metax/score_test.go
Added table-driven tests for LinkDevice.score and LinkDevices.Score, including UUID matches, link-zone combinations, empty and single-device collections, and pairwise scoring.
Collection string output tests
pkg/device/metax/score_test.go
Added tests for empty output, bracket delimiters, and UUID inclusion in non-empty collections.

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

Suggested reviewers: archlitchi

Poem

A rabbit checks each link with care,
Matching zones in tidy pairs.
Empty lists and UUIDs shine,
Scores hop neatly line by line.
Tests now guard the metax trail.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Linked Issues check ✅ Passed The tests cover the linked issue requirements for LinkDevice.score, LinkDevices.Score, and LinkDevices.String.
Out of Scope Changes check ✅ Passed The pull request changes only the scoped test file and does not modify production behavior.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the added unit tests for LinkDevice scoring, which is the main change.
✨ 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.

@coderabbitai
coderabbitai Bot requested a review from archlitchi August 6, 2026 14:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/device/metax/score_test.go`:
- Around line 166-181: Update the test table’s non-empty case to provide the
exact expected device string, then change the assertions in the t.Run block to
compare result with expected for every case. Retain the existing
bracketed-output validation only if needed, while ensuring mismatched device
order or formatting causes the test to fail.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2abd77bb-2953-4241-8c6e-91f343654c83

📥 Commits

Reviewing files that changed from the base of the PR and between 87d9795 and 2e3438d.

📒 Files selected for processing (1)
  • pkg/device/metax/score_test.go

Comment thread pkg/device/metax/score_test.go Outdated
Signed-off-by: aaka24 <yaarsuperstar44@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@archlitchi

Copy link
Copy Markdown
Member

please sync with master to pass the CI

@FouoF

FouoF commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution. These tests cover simple LinkDevice summation and the current formatting of String(). The latter is not a public compatibility contract, so pinning its exact text adds maintenance cost without protecting user behavior. There is no linked defect or reproducible regression, and the current unit-test check is still failing after the requested sync. We are closing this PR. Please choose a maintainer-confirmed issue with an observable failure for the next contribution.

@FouoF FouoF closed this Aug 7, 2026
@aaka24

aaka24 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I understand the concerns regarding testing the String() output and the preference for tests that accompany a maintainer-confirmed bug or reproducible regression. I also appreciate the note about keeping the branch synchronized with master so CI can be evaluated on the latest base.

I'll keep this feedback in mind for future contributions and will focus on issues with observable failures that have maintainer confirmation.

Thanks again for the review and guidance.
@archlitchi @FouoF

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.

test: add unit tests for metax LinkDevice scoring logic

3 participants