Skip to content

Comments

fix(tests): update Jest mock for @octokit/graphql v8 (ESM-only)#5171

Open
SHUBHANSHU602 wants to merge 1 commit intoasyncapi:masterfrom
SHUBHANSHU602:chore/octokit-v8-upgrade
Open

fix(tests): update Jest mock for @octokit/graphql v8 (ESM-only)#5171
SHUBHANSHU602 wants to merge 1 commit intoasyncapi:masterfrom
SHUBHANSHU602:chore/octokit-v8-upgrade

Conversation

@SHUBHANSHU602
Copy link

@SHUBHANSHU602 SHUBHANSHU602 commented Feb 17, 2026

This PR updates the Jest mock for @octokit/graphql in build-dashboard.test.ts.

Since newer versions of @octokit/graphql are ESM-only, Jest was attempting to parse the ESM bundle from node_modules, resulting in:

SyntaxError: Cannot use import statement outside a module

To resolve this, the mock has been updated to use a factory:
jest.mock('@octokit/graphql', () => ({ graphql: jest.fn(), }));

This prevents Jest from loading the actual ESM module and ensures compatibility without modifying Jest configuration or production code.

All tests pass successfully.
Screenshot 2026-02-17 124958

Resolves #3690

Summary by CodeRabbit

  • Tests
    • Improved internal test mocking infrastructure for better test maintainability and reliability.

@netlify
Copy link

netlify bot commented Feb 17, 2026

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 5bfcc7f
🔍 Latest deploy log https://app.netlify.com/projects/asyncapi-website/deploys/6994193b6340d20008068c7b
😎 Deploy Preview https://deploy-preview-5171--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

📝 Walkthrough

Walkthrough

Modified the mock setup for @octokit/graphql in a test file, shifting from a default mock to an explicit module factory pattern that returns an object with a graphql property, introducing a typed alias for improved mock assertions.

Changes

Cohort / File(s) Summary
GraphQL Mock Refactoring
tests/dashboard/build-dashboard.test.ts
Updated @octokit/graphql mock from bare import pattern to explicit module factory with typed alias (mockedGraphql), improving mock structure and assertion specificity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop, hop, the mocks now gleam so bright,
Module factories set things right,
A typed alias makes testing fun,
GraphQL mocking—now it's done!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: updating Jest mock configuration for @octokit/graphql v8 to handle its ESM-only nature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (c02a12f) to head (5bfcc7f).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #5171   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines          796       796           
  Branches       146       146           
=========================================
  Hits           796       796           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@asyncapi-bot
Copy link
Contributor

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 37
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-5171--asyncapi-website.netlify.app/

@SHUBHANSHU602
Copy link
Author

@princerajpoot20 @anshgoyalevil Ready for review.

@SHUBHANSHU602
Copy link
Author

@coderabbitai, does this PR meet the need of Issue ?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any linked issues to this PR
gh pr view 5171 --repo asyncapi/website --json body,title,number,closingIssuesReferences

# Also check if there are any issues related to octokit or ESM
gh issue list --repo asyncapi/website --search "octokit OR graphql OR ESM" --state all --limit 10 --json number,title,state,url

Length of output: 3132


🏁 Script executed:

#!/bin/bash
# Get details of issue `#3690` to understand what needs to be addressed
gh issue view 3690 --repo asyncapi/website --json body,title,number,url,labels

Length of output: 965


🏁 Script executed:

#!/bin/bash
# Check if `@octokit/request` is used in the codebase
rg -n --type ts --type js '@octokit/request' -C2

# Check package.json for both octokit dependencies
cat package.json | jq '.dependencies, .devDependencies' | grep -i octokit -A1 -B1 || echo "No octokit packages found in package.json"

# Also check if there are any imports or usage of both packages
echo "=== Checking for graphql usage ==="
rg -n --type ts --type js "from '@octokit/graphql'" -C1

echo "=== Checking for request usage ==="
rg -n --type ts --type js "from '@octokit/request'" -C1

Length of output: 1024


🏁 Script executed:

#!/bin/bash
# Check PR `#3684` mentioned in the issue for context
gh pr view 3684 --repo asyncapi/website --json title,body,state,closedAt,url

# Also check if there was an actual upgrade to v8 or if PR is just fixing tests for potential upgrade
cat package.json | jq '.dependencies["@octokit/graphql"], .devDependencies["@octokit/graphql"]'

Length of output: 13687


🏁 Script executed:

#!/bin/bash
# Check the actual files changed in PR `#5171`
gh pr diff 5171 --repo asyncapi/website | head -100

# Also check if package.json or package-lock.json were modified
gh pr view 5171 --repo asyncapi/website --json files --jq '.files[].path'

Length of output: 639


@SHUBHANSHU602, this PR partially addresses issue #3690, but doesn't fully complete it. Here's the breakdown:

What's Covered ✅

  • The PR fixes the Jest mock for @octokit/graphql to handle v8's ESM-only format
  • Tests now pass with the updated mock structure
  • The @octokit/request portion of the issue is irrelevant since this package isn't used anywhere in the codebase

What's Missing ❌

The PR does not actually upgrade @octokit/graphql from v7 to v8. Looking at the files changed, only the test file was modified. The package.json still has:

"@octokit/graphql": "^7.0.2"

To Fully Resolve Issue #3690

You need to also update package.json to upgrade the package:

"@octokit/graphql": "^8.2.1"

And run npm install to update the lock file.

Current Status: This PR prepares the test infrastructure for the v8 upgrade but doesn't perform the actual dependency upgrade. Once you add the package.json change, the issue will be fully resolved.

Would you like me to help plan the complete fix including the package upgrade?


🧠 Learnings used
Learnt from: asyncapi-bot
Repo: asyncapi/website PR: 0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch

Learnt from: akshatnema
Repo: asyncapi/website PR: 3101
File: tests/build-rss.test.js:25-27
Timestamp: 2024-11-01T09:55:20.531Z
Learning: In `tests/build-rss.test.js`, replacing `jest.resetModules()` with `jest.resetAllMocks()` in the `afterEach()` block causes errors. It is necessary to use `jest.resetModules()` to reset the module registry between tests in this file.

@asyncapi-bot
Copy link
Contributor

@derberg @princerajpoot20 @akshatnema @sambhavgupta0705 @vishvamsinh28 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋

@SHUBHANSHU602
Copy link
Author

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain
🏁 Script executed:

#!/bin/bash
# Check if there are any linked issues to this PR
gh pr view 5171 --repo asyncapi/website --json body,title,number,closingIssuesReferences

# Also check if there are any issues related to octokit or ESM
gh issue list --repo asyncapi/website --search "octokit OR graphql OR ESM" --state all --limit 10 --json number,title,state,url

Length of output: 3132

🏁 Script executed:

#!/bin/bash
# Get details of issue `#3690` to understand what needs to be addressed
gh issue view 3690 --repo asyncapi/website --json body,title,number,url,labels

Length of output: 965

🏁 Script executed:

#!/bin/bash
# Check if `@octokit/request` is used in the codebase
rg -n --type ts --type js '@octokit/request' -C2

# Check package.json for both octokit dependencies
cat package.json | jq '.dependencies, .devDependencies' | grep -i octokit -A1 -B1 || echo "No octokit packages found in package.json"

# Also check if there are any imports or usage of both packages
echo "=== Checking for graphql usage ==="
rg -n --type ts --type js "from '@octokit/graphql'" -C1

echo "=== Checking for request usage ==="
rg -n --type ts --type js "from '@octokit/request'" -C1

Length of output: 1024

🏁 Script executed:

#!/bin/bash

Check PR #3684 mentioned in the issue for context

gh pr view 3684 --repo asyncapi/website --json title,body,state,closedAt,url

Also check if there was an actual upgrade to v8 or if PR is just fixing tests for potential upgrade

cat package.json | jq '.dependencies["@octokit/graphql"], .devDependencies["@octokit/graphql"]'


Length of output: 13687

🏁 Script executed:

#!/bin/bash

Check the actual files changed in PR #5171

gh pr diff 5171 --repo asyncapi/website | head -100

Also check if package.json or package-lock.json were modified

gh pr view 5171 --repo asyncapi/website --json files --jq '.files[].path'


Length of output: 639

`@SHUBHANSHU602`, this PR **partially addresses** issue `#3690`, but doesn't fully complete it. Here's the breakdown:

## What's Covered ✅
* The PR fixes the Jest mock for `@octokit/graphql` to handle v8's ESM-only format
* Tests now pass with the updated mock structure
* The `@octokit/request` portion of the issue is irrelevant since this package isn't used anywhere in the codebase

## What's Missing ❌
The PR **does not actually upgrade** `@octokit/graphql` from v7 to v8. Looking at the files changed, only the test file was modified. The `package.json` still has:

```json
"@octokit/graphql": "^7.0.2"

To Fully Resolve Issue #3690

You need to also update package.json to upgrade the package:

"@octokit/graphql": "^8.2.1"

And run npm install to update the lock file.

Current Status: This PR prepares the test infrastructure for the v8 upgrade but doesn't perform the actual dependency upgrade. Once you add the package.json change, the issue will be fully resolved.

Would you like me to help plan the complete fix including the package upgrade?

🧠 Learnings used

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain
🏁 Script executed:

#!/bin/bash
# Check if there are any linked issues to this PR
gh pr view 5171 --repo asyncapi/website --json body,title,number,closingIssuesReferences

# Also check if there are any issues related to octokit or ESM
gh issue list --repo asyncapi/website --search "octokit OR graphql OR ESM" --state all --limit 10 --json number,title,state,url

Length of output: 3132

🏁 Script executed:

#!/bin/bash
# Get details of issue `#3690` to understand what needs to be addressed
gh issue view 3690 --repo asyncapi/website --json body,title,number,url,labels

Length of output: 965

🏁 Script executed:

#!/bin/bash
# Check if `@octokit/request` is used in the codebase
rg -n --type ts --type js '@octokit/request' -C2

# Check package.json for both octokit dependencies
cat package.json | jq '.dependencies, .devDependencies' | grep -i octokit -A1 -B1 || echo "No octokit packages found in package.json"

# Also check if there are any imports or usage of both packages
echo "=== Checking for graphql usage ==="
rg -n --type ts --type js "from '@octokit/graphql'" -C1

echo "=== Checking for request usage ==="
rg -n --type ts --type js "from '@octokit/request'" -C1

Length of output: 1024

🏁 Script executed:

#!/bin/bash
# Check PR `#3684` mentioned in the issue for context
gh pr view 3684 --repo asyncapi/website --json title,body,state,closedAt,url

# Also check if there was an actual upgrade to v8 or if PR is just fixing tests for potential upgrade
cat package.json | jq '.dependencies["@octokit/graphql"], .devDependencies["@octokit/graphql"]'

Length of output: 13687

🏁 Script executed:

#!/bin/bash
# Check the actual files changed in PR `#5171`
gh pr diff 5171 --repo asyncapi/website | head -100

# Also check if package.json or package-lock.json were modified
gh pr view 5171 --repo asyncapi/website --json files --jq '.files[].path'

Length of output: 639

@SHUBHANSHU602, this PR partially addresses issue #3690, but doesn't fully complete it. Here's the breakdown:

What's Covered ✅

  • The PR fixes the Jest mock for @octokit/graphql to handle v8's ESM-only format
  • Tests now pass with the updated mock structure
  • The @octokit/request portion of the issue is irrelevant since this package isn't used anywhere in the codebase

What's Missing ❌

The PR does not actually upgrade @octokit/graphql from v7 to v8. Looking at the files changed, only the test file was modified. The package.json still has:

"@octokit/graphql": "^7.0.2"

To Fully Resolve Issue #3690

You need to also update package.json to upgrade the package:

"@octokit/graphql": "^8.2.1"

And run npm install to update the lock file.

Current Status: This PR prepares the test infrastructure for the v8 upgrade but doesn't perform the actual dependency upgrade. Once you add the package.json change, the issue will be fully resolved.

Would you like me to help plan the complete fix including the package upgrade?

🧠 Learnings used

@coderabbitai , I have been asked to fix the failing tests after upgrading the @octokit , so I did the same upgraded it locally and made the required changes , but did not push the upgrade just to maintain the scope of PR , if maintainer ask me I will upgrade that too.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

Tip

For best results, initiate chat on the files or code changes.

@SHUBHANSHU602, thank you for clarifying! That makes perfect sense, and your approach is completely reasonable.

Current Status ✅

You're right to be thoughtful about PR scope. Your current PR:

  • ✅ Fixes the Jest mock to be compatible with v8's ESM-only structure
  • ✅ Ensures tests pass with the updated mock pattern
  • ✅ You've tested locally with v8 and confirmed everything works

Path Forward

There are two valid approaches here:

Option 1: Separate PRs (what you're doing)

  • This PR: Fix test infrastructure only
  • Future PR: Upgrade package.json to v8
  • Advantage: Smaller, focused changes; easier to review and revert if needed

Option 2: Combined PR

  • Include both the test fix AND package.json upgrade
  • Advantage: Fully resolves issue #3690 in one shot

Recommendation

Your current approach is valid, especially since you were specifically asked to fix the failing tests. However, given that:

  • You've already tested with v8 locally
  • The test fix was designed specifically for v8
  • The mock change won't break v7 (it's compatible with both)

You could consider adding the package.json upgrade to this PR to fully close issue #3690, but ultimately this should be the maintainers' decision.

Would you like to wait for maintainer feedback (@princerajpoot20 or @anshgoyalevil) on whether they prefer the upgrade included in this PR or as a separate follow-up?


🧠 Learnings used
Learnt from: akshatnema
Repo: asyncapi/website PR: 3101
File: tests/build-rss.test.js:25-27
Timestamp: 2024-11-01T09:55:20.531Z
Learning: In `tests/build-rss.test.js`, replacing `jest.resetModules()` with `jest.resetAllMocks()` in the `afterEach()` block causes errors. It is necessary to use `jest.resetModules()` to reset the module registry between tests in this file.

@SHUBHANSHU602
Copy link
Author

@princerajpoot20 @anshgoyalevil Please let me know if I should upgrade the @octokit/graphql .If , yes then In this PR or in a follow-up PR.

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

Projects

Status: To Be Triaged

Development

Successfully merging this pull request may close these issues.

fix: Update code to support @octokit/request v9 and @octokit/graphql v8

2 participants