build,test: make test-grpc-packages pass again #352

santigimeno · 2025-08-05T16:49:09Z

No need to install @grpc/proto-loader in Makefile's test-agents-prereqs target.

Summary by CodeRabbit

Chores
- Updated installation steps for test dependencies to simplify package installation.

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target.

coderabbitai · 2025-08-05T16:49:16Z

Walkthrough

The Makefile was updated to modify the test-agents-prereqs target. Specifically, the npm install command for the test/common/nsolid-grpc-agent directory now installs only @grpc/grpc-js, removing @grpc/proto-loader from the installation list. No other changes were made.

Changes

Cohort / File(s)	Change Summary
Makefile Update `Makefile`	Updated `test-agents-prereqs` target to install only `@grpc/grpc-js` and remove `@grpc/proto-loader`.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A tweak in the script, just a line to erase,
No more proto-loader in this Makefile’s embrace.
Only grpc-js hops in with glee,
Simpler installs for you and me!
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch santi/fix_grpc_packages

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce20cc6 and af46941.

📒 Files selected for processing (1)

Makefile (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: santigimeno
PR: nodesource/nsolid#339
File: test/addons/nsolid-elf-utils/nsolid-elf-utils.js:13-14
Timestamp: 2025-07-08T14:46:47.806Z
Learning: In the nsolid test suite, native addon bindings are expected to be built beforehand; tests do not add fs.existsSync guards to skip when bindings are missing.

📚 Learning: in the nsolid test suite, native addon bindings are expected to be built beforehand; tests do not ad...

Learnt from: santigimeno
PR: nodesource/nsolid#339
File: test/addons/nsolid-elf-utils/nsolid-elf-utils.js:13-14
Timestamp: 2025-07-08T14:46:47.806Z
Learning: In the nsolid test suite, native addon bindings are expected to be built beforehand; tests do not add fs.existsSync guards to skip when bindings are missing.

Applied to files:

Makefile

📚 Learning: in nsolid test native addons (e.g., `test/addons/*/binding.cc`), additional javascript-facing argume...

Learnt from: santigimeno
PR: nodesource/nsolid#339
File: test/addons/nsolid-elf-utils/binding.cc:13-28
Timestamp: 2025-07-08T14:48:04.827Z
Learning: In nsolid test native addons (e.g., `test/addons/*/binding.cc`), additional JavaScript-facing argument validation is typically omitted because the tests supply well-formed inputs.

Applied to files:

Makefile

⏰ 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). (6)

GitHub Check: test-macOS
GitHub Check: build-tarball
GitHub Check: test-linux (ubuntu-24.04-arm)
GitHub Check: test-linux (ubuntu-24.04)
GitHub Check: lint-js-and-md
GitHub Check: build-docs

coderabbitai · 2025-08-05T16:51:37Z

Makefile

 	env npm_config_nodedir=$(PWD) $(NODE) ./deps/npm install @opentelemetry/otlp-proto-exporter-base @grpc/grpc-js @grpc/proto-loader --prefix test/common/nsolid-otlp-agent --no-save --no-package-lock
-	env npm_config_nodedir=$(PWD) $(NODE) ./deps/npm install @grpc/grpc-js @grpc/proto-loader --prefix test/common/nsolid-grpc-agent --no-save --no-package-lock
+	env npm_config_nodedir=$(PWD) $(NODE) ./deps/npm install @grpc/grpc-js --prefix test/common/nsolid-grpc-agent --no-save --no-package-lock



💡 Verification agent

🧩 Analysis chain

Verify nsolid-grpc-agent no longer imports @grpc/proto-loader

@grpc/proto-loader has been removed from the install list, so any residual require('@grpc/proto-loader') in test/common/nsolid-grpc-agent/** will crash the tests at runtime.
Please confirm the agent code was cleaned up accordingly.

🏁 Script executed:

#!/bin/bash # Detect stray proto-loader imports after its removal rg -n "@grpc/proto-loader" test/common/nsolid-grpc-agent || true

Length of output: 145

Remove stray @grpc/proto-loader import from nsolid-grpc-agent

@grpc/proto-loader was removed from the install step, but it’s still imported in the agent code, which will lead to runtime failures:

• test/common/nsolid-grpc-agent/server.mjs:4

- import protoLoader from '@grpc/proto-loader';

Please remove this import (and any related usage) to align the code with the updated dependencies.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In the Makefile around lines 1698 to 1700, the dependency @grpc/proto-loader was removed from the install step for nsolid-grpc-agent, but the import statement for it still exists in test/common/nsolid-grpc-agent/server.mjs at line 4. To fix this, remove the import of @grpc/proto-loader from server.mjs and eliminate any code that uses it to ensure the code matches the updated dependencies and prevent runtime errors.

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target. PR-URL: #352 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

santigimeno · 2025-08-05T19:59:54Z

Landed in 766a071

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target. PR-URL: #352 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target. PR-URL: #352 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> PR-URL: #359 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

build,test: make test-grpc-packages pass again

af46941

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target.

santigimeno requested a review from RafaelGSS August 5, 2025 16:49

santigimeno self-assigned this Aug 5, 2025

santigimeno requested a review from juanarbol August 5, 2025 16:49

coderabbitai bot requested changes Aug 5, 2025

View reviewed changes

RafaelGSS approved these changes Aug 5, 2025

View reviewed changes

santigimeno added a commit that referenced this pull request Aug 5, 2025

build,test: make test-grpc-packages pass again

766a071

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target. PR-URL: #352 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

santigimeno closed this Aug 5, 2025

santigimeno deleted the santi/fix_grpc_packages branch August 5, 2025 20:00

santigimeno added a commit that referenced this pull request Aug 25, 2025

build,test: make test-grpc-packages pass again

34deec8

No need to install `@grpc/proto-loader` in Makefile's `test-agents-prereqs` target. PR-URL: #352 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

build,test: make test-grpc-packages pass again #352

build,test: make test-grpc-packages pass again #352

Uh oh!

santigimeno commented Aug 5, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Aug 5, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Aug 5, 2025

Uh oh!

santigimeno commented Aug 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

build,test: make test-grpc-packages pass again #352

build,test: make test-grpc-packages pass again #352

Uh oh!

Conversation

santigimeno commented Aug 5, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

santigimeno commented Aug 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

santigimeno commented Aug 5, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 5, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)