Skip to content

Comments

fix: unit test flakiness#23668

Merged
anikdhabal merged 2 commits intomainfrom
flaky-unit-test
Sep 8, 2025
Merged

fix: unit test flakiness#23668
anikdhabal merged 2 commits intomainfrom
flaky-unit-test

Conversation

@anikdhabal
Copy link
Contributor

No description provided.

@anikdhabal anikdhabal requested a review from a team as a code owner September 8, 2025 03:03
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Caution

Review failed

The head commit changed during the review from d14cdac to 4d57178.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch flaky-unit-test

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.

@graphite-app graphite-app bot requested a review from a team September 8, 2025 03:03
@keithwillcode keithwillcode added the core area: core, team members only label Sep 8, 2025
@vercel
Copy link

vercel bot commented Sep 8, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 8, 2025 3:05am
cal-eu Ignored Ignored Sep 8, 2025 3:05am

@anikdhabal anikdhabal enabled auto-merge (squash) September 8, 2025 03:05
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 (1)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts (1)

1313-1316: LGTM on flakiness fix; small robustness nits for the perf assertion gate

Good call skipping the perf assertion on CI to deflake. Two tiny tweaks to make the local check a bit less brittle and configurable:

  • Use >= to avoid a failure when the ratio equals the threshold.
  • Allow overriding the local threshold via env for slow dev machines.

Apply this diff:

-      if (!process.env.CI) {
-        const minSpeedup = 5; // Assert significant performance improvement (at least 5x faster)
-        expect(speedupRatio).toBeGreaterThan(minSpeedup);
-      }
+      if (!process.env.CI) {
+        const minSpeedup = Number(process.env.MIN_LOCAL_SPEEDUP ?? "5");
+        expect(speedupRatio).toBeGreaterThanOrEqual(minSpeedup);
+      }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 03bc8db and 4d57178.

📒 Files selected for processing (1)
  • packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

Applied to files:

  • packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts

@anikdhabal anikdhabal merged commit 411a14d into main Sep 8, 2025
61 of 62 checks passed
@anikdhabal anikdhabal deleted the flaky-unit-test branch September 8, 2025 03:47
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

E2E results are ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only ready-for-e2e size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants