Skip to content

test coverage for internal/webhooks#230

Merged
gflarity merged 8 commits into
ai-dynamo:mainfrom
gflarity:gflarity/test_coverage_for_internal_webhooks
Nov 7, 2025
Merged

test coverage for internal/webhooks#230
gflarity merged 8 commits into
ai-dynamo:mainfrom
gflarity:gflarity/test_coverage_for_internal_webhooks

Conversation

@gflarity

Copy link
Copy Markdown
Contributor

What type of PR is this?

Testing

What this PR does / why we need it:

Increased test coverage for internal/webhooks

Special notes for your reviewer:

Prompt:

Please create an idiomatic golang unit tests for this specific directory/package. Herea are the instructions:

  • Start by revieiwng the state of test coverage with golang tooling.
  • Be sure to document the fields in the anonymous test structs and how they'll be used below.
  • Each Test* function should also have some concise documentation as well. Each test case should have a brief explanation and expectation explaine. This should be right before the go struct definition for that test if it's table driven tests.
  • Each test should have a name field for cross referencing failures.
  • Rather than having a description field, the description should just be in comments above the name in the struct declaration.
  • Don't forget to run the tests and confirm they're working, but focus on just testing this package as others might have issues.
  • Please avoid creating skipped tests.
  • Refactor code to make it more testable if necessary. However keep these refactors to a minimum and as simple as possible.
  • Specifically hardcoded constants etc are good candidates to be refactor out of testable code. As are interface that can be mocked in a straight forward way. However you must remember to ensure backwards compatibility and new functions rather than changing the signature of public functions. Avoid making these test helpers public though. Only make such refactors when there's a good return on investment.
  • Please ensure coverage is as good as possible using golang tooling.
  • Avoid creating table driven tests when the test function just has a single test in it.
  • Avoid test that don't really test anything.
  • Avoid describing tests as edge cases.
  • Avoid tests for command line arguments. Try to keep test cases unique and adding value (avoid duplicate test cases).

Does this PR introduce a API change?

None

@coderabbitai

coderabbitai Bot commented Oct 23, 2025

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_webhooks branch 2 times, most recently from d22f003 to e55607f Compare October 24, 2025 00:50
@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_webhooks branch from e55607f to 68c6acf Compare November 3, 2025 22:19
@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_webhooks branch from 0c9ed98 to a0a3231 Compare November 6, 2025 15:19

@renormalize renormalize 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.

thanks for the PR @gflarity!

Comment thread operator/internal/webhook/register_test.go Outdated
Comment thread operator/internal/webhook/register_test.go Outdated
Comment thread operator/internal/webhook/register_test.go Outdated
Comment thread operator/internal/webhook/register_test.go Outdated
Comment thread operator/internal/webhook/register_test.go Outdated
Comment thread operator/internal/webhook/admission/pcs/validation/util_test.go Outdated
gflarity and others added 8 commits November 7, 2025 10:17
Signed-off-by: Geoff Flarity <gflarity@nvidia.com>
Signed-off-by: Geoff Flarity <gflarity@nvidia.com>
Signed-off-by: Geoff Flarity <gflarity@nvidia.com>
Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com>
Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com>
Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com>
Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com>
Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com>
Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com>
Signed-off-by: Geoff Flarity <gflarity@nvidia.com>
@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_webhooks branch from ab2ca93 to c804b1b Compare November 7, 2025 15:17
@gflarity

gflarity commented Nov 7, 2025

Copy link
Copy Markdown
Contributor Author

@renormalize thanks for the review. All fixed! PTAL.

@gflarity gflarity requested a review from renormalize November 7, 2025 15:19

@renormalize renormalize 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.

awesome, thanks for the changes @gflarity! Let's get this in now!

@gflarity gflarity merged commit 25ca8fc into ai-dynamo:main Nov 7, 2025
3 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.

3 participants