test: split attachments.test.js#492
Conversation
There was a problem hiding this comment.
The PR splits a large attachments.test.js into focused files, which is a good structural improvement. Several correctness issues were found: silent test assertions inside .catch() blocks that never execute on 2xx responses (making tests always pass), a double-await pattern copied across two files, a wrong EICAR-like mime type in an error message assertion, a misleading comment that contradicts its assertion, and a magic -1 sentinel value in the shared utility function that could produce a malformed Content-Length header if misused.
PR Bot Information
Version: 1.29.2
- LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
328db820-8620-11f1-95b2-b3e78fa5cb02 - Experiment Variant:
control - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
SummaryThe following content is AI-generated and provides a summary of the pull request: test: Split
|
There was a problem hiding this comment.
The PR splits a large test file into four focused ones and introduces a shared uploadDraftAttachment utility. There are several substantive issues across the new files: assertions silently skipped inside .catch() without guaranteeing the handler ran, a double draftModeEdit call before uploadDraftAttachment (which already calls it internally), a scanCleanWaiter that will always time out because no file content is uploaded, and a handful of wrong IsActiveEntity values and misleading comments. Please address the existing review comments before merging.
PR Bot Information
Version: 1.29.2
- File Content Strategy: Full file content
- Experiment Variant:
control - Event Trigger:
pull_request.ready_for_review - Correlation ID:
f32debd0-875c-11f1-97ec-f18fe9fa1cdc - LLM:
anthropic--claude-4.6-sonnet
Uh oh!
There was an error while loading. Please reload this page.