ci: Run CI with specific Parse Server versions#2539
ci: Run CI with specific Parse Server versions#2539mtrezza wants to merge 5 commits intoparse-community:alphafrom
Conversation
|
🚀 Thanks for opening this pull request! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2539 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 6318 6318
Branches 1517 1529 +12
=========================================
Hits 6318 6318 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This is a different thing. It's an attempt (wip) to change the CI to install a specific version of Parse Server, then install the current JS SDK in Parse Server and then run the full Parse Server tests. It's to ensure that a change in the JS SDK won't break a Parse Sever test. As a first step this should only run the SDK tests with different PS versions, but that fails. |
|
I'd rather keep the current way of release the SDK then open a PR on the server and see if it breaks. What happens if the Parse Server test suite does break? We can't update the Parse Server tests as the SDK hasn't been released yet. Not to mention adding 10+ minutes of ci time. Keep the current changes in this PR and add something like |
Sure, this is just meant to detect issues in the SDK, not on the server side. The challenge is that we should detect an issue before making a stable release, because then the bug can affect developers. We don't test with alpha releases of the SDK, so we are releasing an SDK that has not been fully tested with Parse Server. I believe this goes back to parse-community/parse-server#8787, which gives us 2 uses cases:
I'd like to cover (b), which is currently not covered. Another, unrelated issue is to run the SDK CI with multiple versions of Parse Server, which is the current change in this PR. |
Which bugs are you talking about? It's not like installing the SDK along side Parse Server will override the SDK internally used on the server. Besides Parse.Error, Parse._encode, Parse._decode what else does Parse Server use internally that could potentially break it? |
Well, for example these. I'm fine with leaving as is, but I'm afraid it will take a long time until parse-community/parse-server#8787 get's done. This PR instead could focus on what you mentioned and add |
Those haven't changed in years and probably won't change.
Let's start by moving all internal uses of the SDK on Parse Server to a single file so we can see what exactly is used. Then we can decide if we want to use it as a
Default ACL was added in v8 so it's different from v7 |
|
Here we have an example of #2857 breaking the Parse Server CI. Going back and forth to figure out what's causing it. We need to run the Parse Server CI here to detect these issues before release. |
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughCI workflow updated to matrix-test against Parse Server versions 8 and 9 across Node versions, install the target Parse Server per-matrix row, and split test execution into separate install, unit test, and integration test steps while exposing CODECOV_TOKEN and using matrix-driven Node setup and step names. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
No way, what broke? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
114-114: Trailing whitespace after--maxWorkers=4
npm test -- --maxWorkers=4has a trailing space. This is benign but worth cleaning up.♻️ Proposed fix
- run: npm test -- --maxWorkers=4 + run: npm test -- --maxWorkers=4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 114, The run command string "npm test -- --maxWorkers=4 " contains a trailing space; remove the trailing whitespace so it becomes "npm test -- --maxWorkers=4" in the CI step that defines the run value (look for the run: npm test -- --maxWorkers=4 line) to clean up the workflow file.
110-110: Non-deterministic Parse Server version resolved at run time
@7and@8are major-version range specifiers, so the exact7.x.y/8.x.yresolved depends on when npm runs. The-E(--save-exact) flag only pins whichever patch version happens to be current at the moment of the run — it does not make the version deterministic across runs. If a new7.xpatch drops between two CI runs, you get different test targets with no indication in the logs. When diagnosing a failure it will be unclear which exact version was under test.If the intent is to always track "latest within a major," drop
-Eand document it explicitly. If reproducibility is important, pin to a fullMAJOR.MINOR.PATCHversion in the matrix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 110, The workflow step using the command "run: npm i -DE parse-server@${{ matrix.PARSE_SERVER_VERSION }}" is resolving non-deterministic patch versions because "-E" only pins the current resolved patch; either (A) make the matrix values deterministic by replacing the MAJOR-only entries in matrix.PARSE_SERVER_VERSION with exact MAJOR.MINOR.PATCH strings so npm installs a fixed version, or (B) if you intend to always track the latest patch within a major, remove the -E/--save-exact flag and document that behavior; update the matrix or the run command accordingly so the CI target is explicit and reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 113-118: The CI environment variable CI: true is only set for the
"Run integration tests" step, causing the "Run unit tests" step (the npm test --
--maxWorkers=4 step) to run with a different environment; move the CI: true
assignment from the step-level under the "Run integration tests" step to a
job-level env: block so both steps ("Run unit tests" and "Run integration
tests") inherit CI=true and run with consistent test-framework behavior.
- Around line 109-112: The "Install Parse Server ${{ matrix.PARSE_SERVER_VERSION
}}" step is redundant because it runs npm i before the "Install dependencies"
npm ci which then deletes node_modules; fix by switching the order and avoiding
mutation of lockfiles: run the "Install dependencies" (npm ci) first, then run
the "Install Parse Server ${{ matrix.PARSE_SERVER_VERSION }}" step but install
parse-server without updating package.json/package-lock (e.g., use npm install
--no-save parse-server@${{ matrix.PARSE_SERVER_VERSION }} or npm i --no-save
parse-server@...), so the "Install Parse Server" step overlays the installed
tree without touching lockfiles or being erased by npm ci.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 114: The run command string "npm test -- --maxWorkers=4 " contains a
trailing space; remove the trailing whitespace so it becomes "npm test --
--maxWorkers=4" in the CI step that defines the run value (look for the run: npm
test -- --maxWorkers=4 line) to clean up the workflow file.
- Line 110: The workflow step using the command "run: npm i -DE parse-server@${{
matrix.PARSE_SERVER_VERSION }}" is resolving non-deterministic patch versions
because "-E" only pins the current resolved patch; either (A) make the matrix
values deterministic by replacing the MAJOR-only entries in
matrix.PARSE_SERVER_VERSION with exact MAJOR.MINOR.PATCH strings so npm installs
a fixed version, or (B) if you intend to always track the latest patch within a
major, remove the -E/--save-exact flag and document that behavior; update the
matrix or the run command accordingly so the CI target is explicit and
reproducible.
| - name: Install Parse Server ${{ matrix.PARSE_SERVER_VERSION }} | ||
| run: npm i -DE parse-server@${{ matrix.PARSE_SERVER_VERSION }} | ||
| - name: Install dependencies | ||
| run: npm ci |
There was a problem hiding this comment.
npm i -DE followed by npm ci makes the first install entirely redundant
npm i -DE parse-server@${{ matrix.PARSE_SERVER_VERSION }} installs all dependencies (including parse-server) into node_modules, then updates both package.json and package-lock.json. The very next npm ci step deletes node_modules and reinstalls from scratch using those just-updated files. The entire work done by npm i is discarded.
The clean pattern for this use case is to run npm ci first to get a reproducible baseline, then overlay parse-server on top without touching the lock file:
♻️ Proposed fix
- - name: Install Parse Server ${{ matrix.PARSE_SERVER_VERSION }}
- run: npm i -DE parse-server@${{ matrix.PARSE_SERVER_VERSION }}
- - name: Install dependencies
- run: npm ci
+ - name: Install dependencies
+ run: npm ci
+ - name: Install Parse Server ${{ matrix.PARSE_SERVER_VERSION }}
+ run: npm install --no-save parse-server@${{ matrix.PARSE_SERVER_VERSION }}This also removes the unintended side-effect of mutating package.json and package-lock.json in the checkout, and avoids the double-download that contributes to the extra CI time dplewis flagged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 109 - 112, The "Install Parse Server
${{ matrix.PARSE_SERVER_VERSION }}" step is redundant because it runs npm i
before the "Install dependencies" npm ci which then deletes node_modules; fix by
switching the order and avoiding mutation of lockfiles: run the "Install
dependencies" (npm ci) first, then run the "Install Parse Server ${{
matrix.PARSE_SERVER_VERSION }}" step but install parse-server without updating
package.json/package-lock (e.g., use npm install --no-save parse-server@${{
matrix.PARSE_SERVER_VERSION }} or npm i --no-save parse-server@...), so the
"Install Parse Server" step overlays the installed tree without touching
lockfiles or being erased by npm ci.
| - name: Run unit tests | ||
| run: npm test -- --maxWorkers=4 | ||
| - name: Run integration tests | ||
| run: npm run test:mongodb | ||
| env: | ||
| CI: true |
There was a problem hiding this comment.
CI: true env var scoped only to the integration test step, not unit tests
env: CI: true is placed under the npm run test:mongodb step. The preceding npm test step runs without CI=true, so any test-framework behavior gated on that variable (e.g., disabling interactive watch mode, strict warning-as-error modes) will differ between the two steps. Move it to a job-level env: block so both steps share the same environment.
♻️ Proposed fix
timeout-minutes: 30
+ env:
+ CI: true
strategy:
...
steps:
...
- - name: Run integration tests
- run: npm run test:mongodb
- env:
- CI: true
+ - name: Run integration tests
+ run: npm run test:mongodb🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 113 - 118, The CI environment variable
CI: true is only set for the "Run integration tests" step, causing the "Run unit
tests" step (the npm test -- --maxWorkers=4 step) to run with a different
environment; move the CI: true assignment from the step-level under the "Run
integration tests" step to a job-level env: block so both steps ("Run unit
tests" and "Run integration tests") inherit CI=true and run with consistent
test-framework behavior.
|
Note sure, maybe it only broke the parse server test suite, I'm looking into it... |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
125-126:CODECOV_TOKENenv var is redundant alongsidewith: token:The token is already passed to the action via
with: token: ${{ secrets.CODECOV_TOKEN }}on line 124. The addedenv: CODECOV_TOKEN:block duplicates it. codecov-action v4 reads from either source; both together are unnecessary.♻️ Proposed cleanup
- name: Upload code coverage uses: codecov/codecov-action@v4 with: fail_ci_if_error: false token: ${{ secrets.CODECOV_TOKEN }} - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 125 - 126, The workflow sets CODECOV_TOKEN twice causing redundancy: remove the env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} block and rely on the existing with: token: ${{ secrets.CODECOV_TOKEN }} used by the Codecov action (codecov-action v4); ensure only the with: token parameter remains so CODECOV_TOKEN is not duplicated in the job environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 86-88: The CI step "Node 24, Parse Server 8" is pinned to
NODE_VERSION: 24.1.0 which violates Parse Server's engines (requires >=24.11.0);
update the NODE_VERSION value used in that job (refer to the job name "Node 24,
Parse Server 8" and the NODE_VERSION environment variable) to 24.11.0 or a newer
24.x release so npm install/ci won't fail due to engine constraints.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 113-118: The CI variable is only set for the "Run integration
tests" step so the "Run unit tests" step (running npm test) lacks CI=true; move
the env: CI: true entry out of the "Run integration tests" step and hoist it to
the job-level env block so both steps ("Run unit tests" running npm test and
"Run integration tests" running npm run test:mongodb) inherit CI=true.
- Around line 109-112: The CI step currently installs parse-server with npm i
-DE before npm ci which mutates package.json/package-lock and is then discarded;
fix by running npm ci first and then overlay parse-server without touching
lock/package.json: run npm ci in the "Install dependencies" step, then run npm i
parse-server@${{ matrix.PARSE_SERVER_VERSION }} with flags to avoid mutating
files (e.g. --no-save --no-package-lock or --no-save -D --no-package-lock) in
the "Install Parse Server" step so the lockfile remains authoritative.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 125-126: The workflow sets CODECOV_TOKEN twice causing redundancy:
remove the env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} block and rely on
the existing with: token: ${{ secrets.CODECOV_TOKEN }} used by the Codecov
action (codecov-action v4); ensure only the with: token parameter remains so
CODECOV_TOKEN is not duplicated in the job environment.
| - name: Node 24, Parse Server 8 | ||
| NODE_VERSION: 24.1.0 | ||
| PARSE_SERVER_VERSION: 8 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check parse-server v9 and v8 package.json engines field
echo "=== Parse Server v9 (latest in 9.x) ==="
curl -s https://registry.npmjs.org/parse-server | jq '.versions | keys | map(select(startswith("9."))) | .[-1]' -r | head -1 > /tmp/ps9_latest.txt
PS9_VERSION=$(cat /tmp/ps9_latest.txt)
echo "Latest v9: $PS9_VERSION"
curl -s "https://registry.npmjs.org/parse-server/$PS9_VERSION" | jq '.engines // empty'
echo ""
echo "=== Parse Server v8 (latest in 8.x) ==="
curl -s https://registry.npmjs.org/parse-server | jq '.versions | keys | map(select(startswith("8."))) | .[-1]' -r > /tmp/ps8_latest.txt
PS8_VERSION=$(cat /tmp/ps8_latest.txt)
echo "Latest v8: $PS8_VERSION"
curl -s "https://registry.npmjs.org/parse-server/$PS8_VERSION" | jq '.engines // empty'Repository: parse-community/Parse-SDK-JS
Length of output: 367
Node 24.1.0 violates Parse Server's engines constraint (requires >=24.11.0)
Both Parse Server v8 (8.6.2) and v9 (9.3.0-alpha.6) explicitly require Node.js 24 >=24.11.0 <25.0.0. Pinning Node 24.1.0 will cause npm install/npm ci to fail with an engine error, preventing the workflow from running — consistent with the PR author's report that this step currently fails.
Update the pinned version to 24.11.0 or later:
🔧 Proposed fix
- name: Node 24, Parse Server 8
- NODE_VERSION: 24.1.0
+ NODE_VERSION: 24.11.0
PARSE_SERVER_VERSION: 8
- name: Node 24, Parse Server 9
- NODE_VERSION: 24.1.0
+ NODE_VERSION: 24.11.0
PARSE_SERVER_VERSION: 9🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 86 - 88, The CI step "Node 24, Parse
Server 8" is pinned to NODE_VERSION: 24.1.0 which violates Parse Server's
engines (requires >=24.11.0); update the NODE_VERSION value used in that job
(refer to the job name "Node 24, Parse Server 8" and the NODE_VERSION
environment variable) to 24.11.0 or a newer 24.x release so npm install/ci won't
fail due to engine constraints.
|
Ok, it seems to be a npm deduping issue with the push adapter, because the Parse JS SDK is all over the place. We need to get the error codes out of there finally. So not related to #2857. The next big refactor needs to be removing the JS SDK as dependency from all other repos, this is a spaghetti dependency nightmare. |
3 participants
Summary by CodeRabbit