Skip to content

Add missing test coverage for analyzerMode: "server" - #740

Merged
valscion merged 1 commit into
webpack:mainfrom
ronakmaheshwari:main
Sep 25, 2026
Merged

valscion merged 1 commit into
webpack:mainfrom
ronakmaheshwari:main

Conversation

@ronakmaheshwari

@ronakmaheshwari ronakmaheshwari commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #727

Adds missing test coverage for analyzerMode: "server" in test/plugin.js:

  • Tests starting the analyzer server with openAnalyzer: false and analyzerPort: "auto", verifying that the server starts listening and invokes analyzerUrl with the bound host and port.
  • Tests reusing the plugin instance across multiple compiler configurations in server mode, verifying that updateChartData is called with the subsequent compiler's bundle directory (covering the logic introduced in fix: preserve compiler paths for shared plugin instances #725).
  • Ensures proper resource cleanup of HTTP and WebSocket servers across test runs.

What kind of change does this PR introduce?

test

Did you add tests for your changes?

Yes, this PR adds test coverage in test/plugin.js.

Does this PR introduce a breaking change?

No

If relevant, what needs to be documented once your changes are merged or what have you already documented?

N/A (test additions only)

Use of AI
No

@changeset-bot

changeset-bot Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 853d8fd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ronakmaheshwari

Copy link
Copy Markdown
Contributor Author

@valscion I have added the tests which can help you close:- 727. Can you please review it

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 293299cb-5bd1-46e8-b316-eed5a0e744ff

📥 Commits

Reviewing files that changed from the base of the PR and between 6d1ae28 and 6da2211.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • .changeset/use-webpack-infrastructure-logger.md
  • test/plugin.js

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The changes add server-mode test coverage. The tests verify that analyzerUrl receives the server address and that reused plugin instances update chart data for the second compilation. A changeset announces a minor release and documents use of Webpack’s infrastructure logger and deprecation of logLevel.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 6da22

The server-mode coverage verifies the reused-plugin update path, with no concrete merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also migrates the plugin to Webpack's infrastructure logger and adds a changeset that deprecates logLevel in favor of infrastructureLogging. This logger migration is not connected to the te… Remove the infrastructure logger migration and its changeset from this pull request, or link the migration to a separate issue and submit it separately.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #727 requires automated coverage for analyzerMode: "server". The PR adds tests for server startup with openAnalyzer: false and analyzerPort: "auto", verifies the analyzerUrl callback rec…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main test changes for analyzerMode: "server", including server startup and plugin reuse coverage. It is specific and related to the pull request objectives.
Full details: Out of Scope Changes check

Explanation

The PR also migrates the plugin to Webpack's infrastructure logger and adds a changeset that deprecates logLevel in favor of infrastructureLogging. This logger migration is not connected to the test-coverage objective in issue #727.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

⚠️ Fork-based autofix is unavailable. Re-run autofix from a branch in the upstream repository.

@valscion

valscion commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

The PR body and the PR title seems entirely off compared to what this PR has. Looks like a new PR would be better to have as this PR was written from your main branch and not a special branch for this.

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.56%. Comparing base (6d1ae28) to head (853d8fd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #740      +/-   ##
==========================================
+ Coverage   86.75%   87.56%   +0.81%     
==========================================
  Files          17       17              
  Lines        1110     1110              
  Branches      406      406              
==========================================
+ Hits          963      972       +9     
+ Misses        134      126       -8     
+ Partials       13       12       -1     

☔ View full report in Codecov by Harness.
📢 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.

@valscion valscion changed the title Migrate to Webpack logger and improve test coverage Add missing test coverage for analyzerMode: "server" Sep 23, 2026

@valscion valscion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The branch at least needs to be made up-to-date with latest main here so it won't have the changeset and implications that there's some feature work going on in here. Also drop out package-lock.json changes.

@ronakmaheshwari

Copy link
Copy Markdown
Contributor Author

@valscion I have rebased the branch directly on top of the latest upstream/main and dropped both the resurrected changeset file and the package-lock.json changes. The PR now contains only the test coverage additions in test/plugin.js.

@valscion valscion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks!

@valscion
valscion merged commit 6046503 into webpack:main Sep 25, 2026
9 checks passed
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.

Help us add missing test coverage for analyzerMode: "server"

2 participants