Skip to content

improve test coverage for internal/controller#252

Merged
gflarity merged 4 commits into
ai-dynamo:mainfrom
gflarity:gflarity/test_coverage_for_internal_controller
Dec 9, 2025
Merged

improve test coverage for internal/controller#252
gflarity merged 4 commits into
ai-dynamo:mainfrom
gflarity:gflarity/test_coverage_for_internal_controller

Conversation

@gflarity

@gflarity gflarity commented Nov 6, 2025

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/controller

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

shayasoolin
shayasoolin previously approved these changes Nov 10, 2025

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

LGTM

@gflarity gflarity requested a review from shmuel-runai December 8, 2025 15:38
renormalize
renormalize previously approved these changes Dec 9, 2025
@sanjaychatterjee sanjaychatterjee force-pushed the gflarity/test_coverage_for_internal_controller branch from 4cdb46d to c449e0a Compare December 9, 2025 16:00
@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_controller branch from c449e0a to d78f7c6 Compare December 9, 2025 16:14
@gflarity gflarity force-pushed the gflarity/test_coverage_for_internal_controller branch from d78f7c6 to 7abbe25 Compare December 9, 2025 16:21
@gflarity gflarity merged commit 466645a into ai-dynamo:main Dec 9, 2025
4 of 5 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.

4 participants