Skip to content

test: add config CRUD unit tests#597

Open
MD-Mushfiqur123 wants to merge 1 commit into
Agent-Field:mainfrom
MD-Mushfiqur123:tests/config-crud
Open

test: add config CRUD unit tests#597
MD-Mushfiqur123 wants to merge 1 commit into
Agent-Field:mainfrom
MD-Mushfiqur123:tests/config-crud

Conversation

@MD-Mushfiqur123
Copy link
Copy Markdown

@MD-Mushfiqur123 MD-Mushfiqur123 commented May 28, 2026

Closes #391

Adds table-driven Go tests for LocalStorage config CRUD operations: SetConfig, GetConfig, ListConfigs, DeleteConfig.

10 test cases covering:

  • first insert creates version 1
  • upsert increments version, preserves created fields
  • list returns lexical order
  • get missing key returns (nil, nil)
  • delete removes entry
  • delete missing key returns error
  • cancelled context fails fast
  • delete nonexistent config twice
  • list configs when empty
  • multiple upsert round-trip
cd control-plane && go test ./internal/storage/... -run Config -count=1
ok  github.com/Agent-Field/agentfield/control-plane/internal/storage

PR generated by AI agent.

@MD-Mushfiqur123 MD-Mushfiqur123 requested review from a team and AbirAbbas as code owners May 28, 2026 07:01
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Developer seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@AbirAbbas
Copy link
Copy Markdown
Contributor

@MD-Mushfiqur123 thank for the contribution, if you could sign the CLA agreement I can get started on the review

@santoshkumarradha
Copy link
Copy Markdown
Member

Quick heads-up: this is going to conflict with #606. Both PRs add control-plane/internal/storage/config_storage_db_test.go for issue #391, and the newer PR covers the same CRUD surface plus cancellation/deadline paths.

Please coordinate so we keep one version of this test file. Otherwise one of the two PRs will be stale even before CLA is sorted out.

Copy link
Copy Markdown
Member

@santoshkumarradha santoshkumarradha left a comment

Choose a reason for hiding this comment

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

Thanks for filling in config-storage coverage here. I pulled the branch locally and checked it against main, and the main issue is that this file mostly duplicates scenarios we already exercise in existing tests.

TestLocalStorageConfigLifecycle already covers the SQLite path for insert, update/version bump, lexical list order, delete, missing delete, and canceled context. We also already cover the postgres branch in coverage_storage_clinch_test.go. Adding a second full CRUD suite for the same behavior makes this area harder to maintain without giving us much new signal.

Can you tighten this down to only the cases that are genuinely missing from current coverage, or extend the existing test(s) instead of introducing a parallel suite? Once that overlap is removed, I’m happy to take another look.


"github.com/stretchr/testify/require"
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most of this new file overlaps with coverage we already have on main. TestLocalStorageConfigLifecycle in local_vector_config_pubsub_test.go already exercises the SQLite config CRUD path, and coverage_storage_clinch_test.go covers the postgres branch too. Can we fold any truly missing assertions into those existing tests instead of adding a second end-to-end CRUD suite here?

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.

[Control Plane] Add tests for LocalStorage config CRUD

4 participants