-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
ci: Fix MongoDB test instance not terminated after running tests #9860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Fix MongoDB test instance not terminated after running tests #9860
Conversation
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe PR updates MongoDB test orchestration by switching npm scripts to use mongodb-runner exec, removes pretest/posttest hooks and old runner scripts, adds a coverage:mongodb script, bumps mongodb-runner from 5.8.3 to 5.9.3, and updates the CI workflow to call the new coverage:mongodb script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/CI
participant NPM as npm scripts
participant MR as mongodb-runner
participant App as Test Runner
Dev->>NPM: npm run test / coverage:mongodb
NPM->>MR: exec -- mongo start + run command
MR->>App: npm run testonly / coverage
App-->>MR: exit code
MR-->>NPM: propagate exit code
NPM-->>Dev: success/failure
rect rgba(220,240,255,0.4)
note over NPM,MR: New flow uses single-process<br/>mongodb-runner exec (no pre/post hooks)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
I will reformat the title to use the proper commit message syntax. |
@mtrezza for current active contributors, they need to run the "clean:mongodb" script to prune some old ref, then the upgrade of the package seems to fix the issue 🎉 |
There was a problem hiding this 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 (2)
CONTRIBUTING.md (1)
518-519
: Good addition; consider surfacing this under Troubleshooting, too.The note is helpful and scoped. To improve discoverability, mirror this line under the “Troubleshooting” section (Lines 217–231) with the same command.
package.json (1)
140-146
: Nice: dedicated cleanup script. Optional: add a “clean:testenv” combo.A convenience alias helps when CI/local runs leave stale state. Example:
"posttest": "cross-env mongodb-runner stop --all", "clean:mongodb": "cross-env mongodb-runner prune", + "clean:testenv": "cross-env mongodb-runner stop --all && cross-env mongodb-runner prune", "coverage": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=8.0.4} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} TESTING=1 nyc jasmine",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
CONTRIBUTING.md
(1 hunks)package.json
(2 hunks)
⏰ 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). (12)
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Docker Build
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 16, PostGIS 3.5
🔇 Additional comments (1)
package.json (1)
106-106
: Provide upstream changelog/commit proving ESRCH ('kill ESRCH') fix in mongodb-runner v5.9.3No release note or changelog entry explicitly mentions an ESRCH / "kill ESRCH" cleanup fix in v5.9.3. Add a direct link to the upstream changelog/commit/PR in the PR description (or a short note here) showing the fix so reviewers can verify the rationale for the version bump.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9860 +/- ##
=======================================
Coverage 93.01% 93.01%
=======================================
Files 187 187
Lines 15096 15096
Branches 174 174
=======================================
Hits 14041 14041
Misses 1043 1043
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
note: it seems that the update don't entirely fix the issue, i need to check additional things |
okay @mtrezza switched the test system to mongo exec, in case of test failure mongo process is correctly killed, i removed some useless scripts now. Tell me what you think ! Feel free to checkout my PR :) |
Note: @mtrezza An even better approach could be to use the dynamically generated Mongo URI (currently forced to port 27017). This way, even if a developer has another process running, or runs two tests in parallel, it would still work. However, this is quite a specific case. I think the priority here is to have a clean and easy contributing system, and right now the MongoDB runner issue is tedious during the testing phase. |
nice it seems that everything pass :) |
ping @parse-community/server-review |
Re-running docker CI |
Pull Request
Issue
Closes: Mongodb runner clean issue
Approach
Update the package, and switch to mongodb exec to automatically kill mongo process
Tasks
Summary by CodeRabbit