Skip to content

fix(operator): OPS-9038 use promoted fields in the DGD resource syncer test - #17453

Merged
dmitry-tokarev-nv merged 1 commit into
mainfrom
dtokarev/ops-9038-operator-embedlit-lint
Oct 28, 2026
Merged

dmitry-tokarev-nv merged 1 commit into
mainfrom
dtokarev/ops-9038-operator-embedlit-lint

Conversation

@dmitry-tokarev-nv

@dmitry-tokarev-nv dmitry-tokarev-nv commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Linear: OPS-9038

Summary

The operator job in Pre Merge fails on main. golangci-lint v2.15.0 reports two modernize findings:

internal/controller/dgd_resource_syncer_test.go:106:6: embedlit: embedded field type can be removed from struct literal (modernize)
internal/controller/dgd_resource_syncer_test.go:109:6: embedlit: embedded field type can be removed from struct literal (modernize)

Validation

All local runs used the golang:1.27.3 image with the CI digest (sha256:921f6112…), golangci-lint v2.15.0, linux/arm64 and --network none.

  • golangci-lint run on main (cffc4fa75c) gives the two findings above. This branch gives 0 issues. main before refactor(operator): share ownership-safe deletion for DGD auxiliary resources #16910 (95f1b12d0b) also gives 0 issues, so no other modernize finding remains.
  • On main, the arm64 lint output is the same as the amd64 output of the CI job.
  • go test ./internal/controller/ -run '^TestDGDResourceSyncerDeleteIfControlledBy$' passes 8 of 8 subtests on main and on this branch.
  • A mutant that gives both DGDs the same UID fails "object controlled by another owner is left alone". So the test still reads the UIDs that the new literals set.
  • go vet ./internal/controller/ and gofmt -l are clean on this branch.
  • pre-commit run --all-files passes all 40 hooks.
  • Not run locally: make check, the full make test, make envtest, and amd64. The operator job of this PR runs them.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Simplified test fixture declarations; test cases and assertions are unchanged.

…r test

The operator lint job fails on main. golangci-lint v2.15.0 reports two
modernize/embedlit findings in dgd_resource_syncer_test.go, lines 106
and 109. #16910 added the file with `ObjectMeta: metav1.ObjectMeta{...}`
literals. Its last CI run came before #16771 enabled modernize and
rewrote all such literals in the operator.

Set Name, Namespace and UID through the promoted fields, as #16771 did,
and remove the metav1 import, which has no other use in the file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
@dmitry-tokarev-nv
dmitry-tokarev-nv requested a review from a team as a code owner October 27, 2026 21:26
@github-actions github-actions Bot added fix deployment::k8s Relates to dynamo deployment in kubernetes labels Oct 27, 2026
@dmitry-tokarev-nv
dmitry-tokarev-nv enabled auto-merge (squash) October 27, 2026 21:29
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between cffc4fa and 726c831.

Files selected for processing: 1
  • deploy/operator/internal/controller/dgd_resource_syncer_test.go

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

Walkthrough

The test removes the metav1 import and sets Name, Namespace, and UID directly on two DynamoGraphDeployment fixtures. Test cases and assertions remain unchanged.

Changes

Test fixture metadata cleanup

Layer / File(s) Summary
Update test fixture metadata fields
deploy/operator/internal/controller/dgd_resource_syncer_test.go
The test removes the unused metav1 import. Both fixtures set Name, Namespace, and UID directly instead of using nested ObjectMeta literals.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: Minimal · up to 726c8

This is a test-only lint cleanup with no production behavior change. The compile-failure concern was refuted, so the change is safe to merge.

Pre-merge checks: Passed 4 · Failed 1

Failed 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%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the operator test fix, the use of promoted fields, and the related issue.
Description check Passed The description explains the lint failure, the code change, the issue reference, and validation results. It does not use every template heading, but it provides the required issue reference and suffic…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the lint failure, the code change, the issue reference, and validation results. It does not use every template heading, but it provides the required issue reference and sufficient technical detail.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@dmitry-tokarev-nv
dmitry-tokarev-nv merged commit 37f2f0a into main Oct 28, 2026
202 of 204 checks passed
@dmitry-tokarev-nv
dmitry-tokarev-nv deleted the dtokarev/ops-9038-operator-embedlit-lint branch October 28, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment::k8s Relates to dynamo deployment in kubernetes fix size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants