Skip to content

Conversation

@grunch
Copy link
Member

@grunch grunch commented Sep 17, 2025

Change back after #697 is merge

Summary by CodeRabbit

  • Tests

    • Adjusted bot initialization test to remove a redundant job assertion, preserving the cron timing while improving determinism and reducing intermittent CI failures.
  • Style

    • Reformatted a conditional in image handling for readability; no functional or behavioral changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The bot initialization test stops stubbing Jobs.deleteCommunity, reducing scheduledJob invocations from 8 to 7; a formatting-only change was made to a base64-detection conditional in util/imageCache.ts. No public/exported signatures were changed.

Changes

Cohort / File(s) Summary
Bot initialization tests
tests/bot/bot.spec.ts
Commented out the mock for Jobs.deleteCommunity in the proxyquire mock; updated assertion for scheduled job invocation count from 8 to 7; first cron expression remains */10 * * * *.
Image cache formatting
util/imageCache.ts
Reformatted the if condition in convertImageToBase64 into a multi-line form; no behavioral or control-flow changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Catrya

Poem

A hop, a nibble, one fewer chore,
Seven little jobs now patter the floor.
I twitch my whiskers at tidy code art,
Crons still tick while carrots impart.
Hooray for tests — I bound and depart! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely describes the primary intent of the changeset—making the test pass after the deleteCommunity behavior was disabled, which matches the change in tests/bot/bot.spec.ts that removes the deleteCommunity stub; it is focused, short, and not noisy. The title need not cover minor formatting changes elsewhere, so it meets the PR title criteria.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
tests/bot/bot.spec.ts (2)

307-315: Remove commented-out stub to avoid dead code drift

The deleteCommunity stub is now commented out. If the job is disabled upstream, drop the commented line to keep the test doubles clean. If you plan to re-enable via a follow-up PR, prefer adding a brief TODO referencing the PR instead of leaving code commented out.

-        // deleteCommunity: sinon.stub().resolves(),

469-477: Avoid brittle assertion on total number of scheduled jobs

start.ts registers 8 scheduleJob calls (not 7); assert presence of required cron(s) instead of the total count.

File: tests/bot/bot.spec.ts Lines: 469-477

Proposed change:

-    expect(scheduleStub.scheduleJob.callCount).to.be.equal(7);
+    const crons = scheduleStub.scheduleJob.getCalls().map(c => c.args[0]);
+    expect(crons).to.include('*/10 * * * *'); // attemptPendingPayments
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91ca64e and 7ab53f0.

📒 Files selected for processing (1)
  • tests/bot/bot.spec.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context

Files:

  • tests/bot/bot.spec.ts
tests/**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Write tests in TypeScript using Mocha and Chai with the *.spec.ts naming under tests/

Files:

  • tests/bot/bot.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
PR: lnp2pBot/bot#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts

Applied to files:

  • tests/bot/bot.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: ci_to_main
  • GitHub Check: ci_to_main

@grunch grunch merged commit ad90068 into main Sep 17, 2025
7 checks passed
@grunch grunch deleted the fix/delete-community-test branch September 17, 2025 13:19
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