Test the Mongo integration with the real driver#1531
Conversation
|
Hi @unflxw, We've found some issues with your Pull Request.
|
There was a problem hiding this comment.
Pull request overview
This PR improves coverage of the Mongo Ruby driver integration by exercising Appsignal::Hooks::MongoMonitorSubscriber against real Mongo::Monitoring::Event objects (without needing a MongoDB server), and wires CI to run those specs under a dedicated mongo.gemfile.
Changes:
- Add
DependencyHelper.mongo_present?and gate Mongo-related specs based on the mongo gem being present. - Rewrite Mongo subscriber specs to use real
Mongo::Monitoring::Event::{CommandStarted,CommandSucceeded,CommandFailed}objects and assert on recorded transaction events/metrics. - Add
gemfiles/mongo.gemfileand extend the build matrix + generated GitHub Actions workflow to run the mongo job.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/helpers/dependency_helper.rb | Adds mongo_present? helper for conditional specs. |
| spec/lib/appsignal/integrations/mongo_ruby_driver_spec.rb | Switches to real Mongo monitoring events and new assertions for recorded events/metrics. |
| spec/lib/appsignal/hooks/mongo_ruby_driver_spec.rb | Adjusts dependency expectations when the real driver is loaded via mongo.gemfile. |
| gemfiles/mongo.gemfile | Introduces a gemfile that includes the mongo gem for CI/spec gating. |
| build_matrix.yml | Adds mongo to the build matrix gem variants. |
| .github/workflows/ci.yml | Regenerates CI workflow to include mongo gemfile jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot resolve the merge conflicts in this pull request |
Resolved the merge conflict by merging |
The subscriber specs fed the monitoring callbacks hand-built doubles, so the integration was never exercised against the driver's real event API. Build real `Mongo::Monitoring::Event` objects instead and drive the started/succeeded/failed callbacks; their constructors don't open a connection, so no MongoDB server is needed. Assert on the recorded event rather than on internal extension calls. This needs the gem, so add a mongo gemfile and build matrix entry and gate the specs on it. [skip changeset]
39743ac to
885b0fc
Compare
Part of #1515, spun as a separate PR to make reviewing easier. This extracts a test-quality improvement the OpenTelemetry work drags in, so it can be reviewed and validated independently of collector mode.
Test the Mongo integration with the real driver
The subscriber specs fed the monitoring callbacks hand-built doubles, so the integration was never exercised against the driver's real event API. Build real
Mongo::Monitoring::Eventobjects instead and drive the started/succeeded/failed callbacks; their constructors don't open a connection, so no MongoDB server is needed. Assert on the recorded event rather than on internal extension calls.This needs the gem, so add a mongo gemfile and build matrix entry and gate the specs on it.