Skip to content

feat: implement issue #528 — SonarCloud: test assertion quality (S5906) #548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
don-petry wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: implement issue #528 — SonarCloud: test assertion quality (S5906) #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/calendar-to-sheets/tests/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1509,10 +1509,10 @@ describe('Checkpoint logic (GAS only)', () => {
const configs = freshCode.getConfigs()

// Should fall back to legacy mode when array is empty
expect(configs.length).toBe(1)
expect(configs).toHaveLength(1)
expect(configs[0].sheetName).toBe('Sheet1')
expect(configs[0].spreadsheetId).toBe(null)
expect(configs[0].calendarId).toBe(null)
expect(configs[0].spreadsheetId).toBeNull()
expect(configs[0].calendarId).toBeNull()

delete global.SYNC_CONFIGS
})
Expand Down
2 changes: 1 addition & 1 deletion src/gmail-ai-classifier/tests/performance-scalability.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('Performance and Scalability Benchmarks', () => {
const processed = processThreadBatch(threads, config, services)
const duration = Date.now() - startTime

expect(processed.length).toBe(BATCH_SIZE)
expect(processed).toHaveLength(BATCH_SIZE)

@codeant-ai codeant-ai Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The benchmark only verifies the number of returned entries, so an implementation that duplicates one result, associates results with the wrong threads, or returns 100 unclassified entries would still pass. Assert that each result corresponds to the input thread and has the expected classified status and label so the throughput test also validates the batch-processing contract. [incomplete implementation]

Severity Level: Major ⚠️ 
- ⚠️ Batch benchmark can pass incorrect per-thread results.
- ⚠️ Thread-to-result association regressions may go undetected.
- ⚠️ Classification status and label regressions lack coverage.

Use CodeAnt Skill

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/gmail-ai-classifier/tests/performance-scalability.test.js
**Line:** 124:124
**Comment:**
	*Incomplete Implementation: The benchmark only verifies the number of returned entries, so an implementation that duplicates one result, associates results with the wrong threads, or returns 100 `unclassified` entries would still pass. Assert that each result corresponds to the input thread and has the expected classified status and label so the throughput test also validates the batch-processing contract.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

expect(duration).toBeLessThan(100) // 100 items benchmarked under 100ms
})
})
Loading