Skip to content

Conversation

@snomiao
Copy link
Member

@snomiao snomiao commented Nov 5, 2025

Summary

This PR fixes test failures in GitHub Actions CI and improves test code quality with a cleaner mocking pattern.

Root Cause

The tests were failing because:

  1. The database module (src/db/index.ts) uses top-level await to initialize MongoDB connection
  2. Jest-style mocks (jest.mock()) were not being applied before module imports in Bun's test runner
  3. Tests attempted to connect to MongoDB even though the database was supposed to be mocked

Changes Made

Test Infrastructure (Initial Fix)

  • Migrated from Jest mocks to Bun's native mocking: Replaced jest.mock() with mock.module() from bun:test
  • Dynamic imports: Used await import() to ensure mocks are set up before modules are loaded
  • MONGODB_URI environment variable: Added in src/test/msw-setup.ts to prevent production database connections

Code Quality Improvements (Refactoring)

  • Introduced factory pattern: Created createMockState() factory function for fresh test state
  • Closure-based mocking: Implemented createMockCollection() that references mockState via closure
  • Declarative state management: Replaced imperative mock reassignment with simple state reset in beforeEach
  • Per-test customization: Added findOneImpl and findOneAndUpdateImpl for easy test-specific behavior
  • Reduced boilerplate: Eliminated verbose manual mock function reassignments

Files Modified

  • app/tasks/gh-desktop-release-notification/index.spec.ts - Converted to Bun-native mocking + factory pattern
  • app/tasks/gh-core-tag-notification/index.spec.ts - Converted to Bun-native mocking + factory pattern
  • src/test/msw-setup.ts - Added MONGODB_URI environment variable for tests

Test Corrections

  • Fixed expected text formats to match actual implementation
  • Updated repo name format from `"desktop"` to `"Comfy-Org/desktop"` in assertions

Test Results

✅ All 16 tests now pass:

  • 10 tests in gh-desktop-release-notification
  • 6 tests in gh-core-tag-notification

```
16 pass
0 fail
23 expect() calls
Ran 16 tests across 2 files. [237.00ms]
```

Benefits of Refactoring

  1. Less verbose: No need to reassign mock functions in `beforeEach`
  2. More functional: State creation is pure and predictable
  3. More flexible: Easy per-test customization without mutation
  4. Better maintainability: Cleaner separation of concerns
  5. Consistent pattern: Same approach across both test files

Related Issue

Fixes failing tests from: https://github.com/Comfy-Org/Comfy-PR/actions/runs/19085256036/job/54523916169

🤖 Generated with Claude Code

…core-tag-notification

This commit fixes test failures that occurred due to MongoDB connection
attempts during test execution. The root cause was that the database
module was being imported and initialized (with top-level await) before
test mocks could be applied.

Changes made:

1. **Test Mocking Strategy**: Converted from jest.mock() to Bun's
   mock.module() with dynamic imports, ensuring mocks are set up
   before modules are loaded.

2. **Test Files Updated**:
   - app/tasks/gh-desktop-release-notification/index.spec.ts
   - app/tasks/gh-core-tag-notification/index.spec.ts

3. **Mock Implementation**: Replaced Jest-style mocks with Bun-native
   mocking patterns using tracking arrays to monitor function calls.

4. **Test Fixes**: Corrected expected text formats to match actual
   implementation (repo name format: "Comfy-Org/desktop" vs "desktop").

5. **Environment Setup**: Added MONGODB_URI to test setup to prevent
   connection attempts to production databases during tests.

All 16 tests now pass successfully without attempting MongoDB connections.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
comfy-pr Ready Ready Preview Comment Nov 5, 2025 9:06pm

snomiao and others added 2 commits November 5, 2025 10:29
…ease-notification

Replace verbose manual mock reassignment in beforeEach with a cleaner factory pattern:
- Introduce createMockState() factory function to generate fresh state
- Use createMockCollection() that references mockState via closure
- Replace imperative mock reassignment with declarative state reset
- Support per-test customization via findOneImpl/findOneAndUpdateImpl
- Reduce boilerplate and improve readability

Benefits:
- Less verbose: no need to reassign mock functions in beforeEach
- More functional: state creation is pure and predictable
- More flexible: easy per-test customization without mutation
- Better maintainability: cleaner separation of concerns

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…tification

Apply the same factory pattern improvements as gh-desktop-release-notification:
- Introduce createMockState() factory for fresh test state
- Use createMockCollection() with closure over mockState
- Replace imperative reassignments with declarative state reset
- Support per-test customization via findOneImpl/findOneAndUpdateImpl
- Simplify beforeEach by removing manual mock function reassignments

This brings consistency across the codebase and improves test maintainability.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add pull_request trigger to test workflow so tests run on PRs targeting main branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@socket-security
Copy link

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm safer-buffer is 94.0% likely obfuscated

Confidence: 0.94

Location: Package overview

From: ?npm/d3@7.9.0npm/safer-buffer@2.1.2

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/safer-buffer@2.1.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

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.

2 participants