Skip to content

test: add TestValidateConfig() - #3170

Closed
Kevindua26 wants to merge 1 commit into
Project-HAMi:masterfrom
Kevindua26:test/3154-add-validateConfig
Closed

Kevindua26 wants to merge 1 commit into
Project-HAMi:masterfrom
Kevindua26:test/3154-add-validateConfig

Conversation

@Kevindua26

@Kevindua26 Kevindua26 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adds unit test coverage for the validateConfig() function in config.go. This function validates raw configuration before device initialization, checking that at least one device backend is configured, NVIDIA defaultCores is 0-100 percentage, and NVIDIA defaultMemory is not negative.

Which issue(s) this PR fixes:
Fixes #3154

Special notes for your reviewer:

This PR was prepared with assistance from GitHub Copilot.

Does this PR introduce a user-facing change?:
No

Summary by CodeRabbit

  • Tests
    • Added coverage for scheduler configuration validation, including invalid core percentages, negative memory values, empty configurations, and valid settings.

Signed-off-by: Kevindua26 <kevindua26@gmail.com>
@hami-robot hami-robot Bot added the kind/feature new function label Oct 5, 2026
@hami-robot

hami-robot Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Kevindua26
Once this PR has been reviewed and has the lgtm label, please assign archlitchi 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

@github-actions github-actions Bot removed the kind/feature new function label Oct 5, 2026
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
CLAUDE.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a2825083-7d2e-4e8c-bef0-fdbdc2f1b63e
📥 Commits

Reviewing files that changed from the base of the PR and between 717f016 and 110db39.

📒 Files selected for processing (1)
  • pkg/scheduler/config/config_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Added table-driven tests for validateConfig. The tests check empty configuration, out-of-range NVIDIA default cores, negative NVIDIA default memory, and a valid NVIDIA configuration.

Changes

Configuration Validation

Layer / File(s) Summary
Test validateConfig
pkg/scheduler/config/config_test.go
Added tests that assert errors for empty configuration and invalid NVIDIA defaults, and verify that a valid NVIDIA configuration succeeds.

Priority: ⬇️ Low

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

Change: Other

Suggested reviewers: moezdil

Merge Risk: ⚪ Minimal · up to 110db

The change adds configuration-validation tests without changing runtime behavior; no merge-blocking risk is identified.

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. 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 and concisely identifies the addition of the TestValidateConfig() unit test, which is the main change.
Linked Issues check ✅ Passed Issue #3154 requests TestValidateConfig coverage for an empty configuration, NVIDIA defaultCores outside 0–100%, and negative NVIDIA defaultMemory. The added table-driven test checks each case and che…
Out of Scope Changes check ✅ Passed The pull request adds only TestValidateConfig in pkg/scheduler/config/config_test.go. The test directly supports issue #3154. No unrelated changes appear in the whole-PR diff.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks each setting with care
Empty and invalid values meet a test
Core limits stay within their bounds
Memory below zero gets a check
A valid config passes the way
Then hops along its merry way

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

@coderabbitai
coderabbitai Bot requested a review from moezdil October 5, 2026 12:43
@codecov

codecov Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 77.33% <ø> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Kevindua26 Kevindua26 closed this Oct 5, 2026
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.

test: add test to validateConfig() in config.go

1 participant