test(#2511): strengthen agent-field assertions after ADR-0045 Phase 4#2515
test(#2511): strengthen agent-field assertions after ADR-0045 Phase 4#2515fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Finding 1: In TestNewOrgConfig, change assert.Empty to assert.Nil for cfg.Agents. Since NewOrgConfig no longer sets the Agents field, the value should be nil (Go zero value), not merely empty. This enforces the stronger invariant that matters for YAML omitempty serialization behavior. Finding 2: In TestConfigRepoLayer_Install_CreatesRepo, add an integration-level assertion that reads back the written config.yaml content and verifies it does not contain an "agents:" key. This confirms the end-to-end behavior that Install() produces an agents-free config file. Finding 3 (NewOrgConfig options struct) is deferred per the issue as a design improvement, not a blocker. Note: pre-commit could not run due to shellcheck-py network error in sandbox (exit 3). The post-script runs pre-commit authoritatively on the runner. Closes #2511
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://a94eeab9-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 4:04 PM UTC · Completed 4:13 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Looks good to me. Low
Labels: Test-only change in the install config-repo layer addressing prior review findings. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| // ADR-0045 Phase 4: NewOrgConfig no longer sets the agents field, | ||
| // so the written config.yaml must not contain an agents: key. | ||
| assert.NotContains(t, configContent, "agents:", "config.yaml should not contain agents: block after ADR-0045 Phase 4") | ||
| } |
There was a problem hiding this comment.
[low] test-adequacy
The assert.NotContains(t, configContent, "agents:") assertion could pass vacuously if configContent is empty. Adding assert.NotEmpty(t, configContent) before the NotContains check would guard against this silent pass-through.
Suggested fix: Add assert.NotEmpty(t, configContent, "config.yaml content should not be empty") before the NotContains assertion.
fullsend-ai-review
Bot
added
ready-for-merge
|
/fs-fix Rebase and address the review issues. |
|
🤖 Finished Fix · ❌ Failure · Started 4:34 PM UTC · Completed 4:40 PM UTC |
|
/fs-fix Rebase and address the review issues. |
|
🤖 Finished Fix · ❌ Failure · Started 6:49 PM UTC · Completed 6:52 PM UTC |
1 participant
Finding 1: In TestNewOrgConfig, change assert.Empty to assert.Nil for cfg.Agents. Since NewOrgConfig no longer sets the Agents field, the value should be nil (Go zero value), not merely empty. This enforces the stronger invariant that matters for YAML omitempty serialization behavior.
Finding 2: In TestConfigRepoLayer_Install_CreatesRepo, add an integration-level assertion that reads back the written config.yaml content and verifies it does not contain an "agents:" key. This confirms the end-to-end behavior that Install() produces an agents-free config file.
Finding 3 (NewOrgConfig options struct) is deferred per the issue as a design improvement, not a blocker.
Note: pre-commit could not run due to shellcheck-py network error in sandbox (exit 3). The post-script runs pre-commit authoritatively on the runner.
Closes #2511
Post-script verification
agent/2511-review-findings-adr0045)619c19d688c0aa3c25af0be4fd8b07cbc8099241..HEAD)