Nextcloud 34 prep (CI/README) + Postgres integration-test stability#54
Conversation
…il nextcloud:34 image publishes (GA 2026-06-09)
|
Warning Review limit reached
More reviews will be available in 44 minutes and 19 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughClarifies CI wording about testing the two newest Nextcloud releases (currently 32 and 33) and notes Nextcloud 34 will be added when its Docker image is published. Adds idempotent, timestamp-stable comment seeding and updates integration tests for pagination, event creation, and user-status cleanup. ChangesCI Version Support Documentation
Integration test stability and pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
(AI) Ready for review — all CI green (lint+types, unit on 3.12/3.13/3.14, and integration + session-cache + user-permissions on NC 32 & 33). Scope: intentionally minimal. Local verification (dev instances, PostgreSQL):
The single failure is identical on both versions — |
oleksandr-nc
changed the title
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/test_pagination.py`:
- Around line 60-69: The test currently fetches only the first page (limit=500)
via nc_mcp.call("get_events") and builds existing_summaries from result["data"],
which misses events beyond that page; change the helper to paginate: repeatedly
call nc_mcp.call("get_events") (using the response's pagination token/has_more
flag and whatever next_cursor/offset the API returns) appending result["data"]
into existing_summaries until has_more is false or all expected summaries (e.g.
all "Pagination Test Event ###") are found, then proceed with assertions; update
the logic that reads result["has_more"] and result["next_cursor"] (or
equivalent) and merge summaries rather than capping at a single 500-item call.
In `@tests/integration/test_search.py`:
- Around line 112-130: The test currently casts first["cursor"] and
data["cursor"] to str even when has_more is true, which masks None as "None";
update the loop in tests/integration/test_search.py (the block using first,
data, cursor, next_cursor, and has_more) to assert that a cursor is present
whenever has_more is true (e.g., assert first["cursor"] is not None before
assigning cursor and assert data["cursor"] is not None whenever data["has_more"]
is True) so the test fails if the API advertises more pages without returning a
cursor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb7420ac-0de8-400a-acf2-2ab90fa85e14
📒 Files selected for processing (4)
scripts/seed_pagination_data.pytests/integration/test_pagination.pytests/integration/test_search.pytests/integration/test_user_status.py
1 participant
Summary
Two related changes.
1. Nextcloud 34 prep (CI + README)
NC34 GA is 2026-06-09. The official
nextcloud:34Docker image isn't published yet (docker manifest inspect nextcloud:34→ no such manifest; Docker Hub's newest is33.0.3), so adding34to the integration matrix now would fail CI on image pull.tests-integration.yml): matrix stays["32", "33"]— the two newest released versions. Added a dated comment marking where to add"34"/ drop"32"once the image publishes.The actual flip (matrix →
["33", "34"], badge →33 | 34) lands at GA, following the rolling-window precedent (2c66ff1, which replaced EOL NC31 with NC33). NC32 keeps security support until ~Sept 2026, so it stays until 34 is wired in.2. Integration test stability on PostgreSQL
The dev instances run PostgreSQL; these remove flakiness that surfaces there (and harden the suite for the bare-metal-Postgres CI effort, #28):
test_search.py::test_search_pagination— NC'sFilesSearchProviderissues its DB query without anORDER BY, so on PostgresLIMIT/OFFSETpages can skip or overlap rows; full enumeration of the seeded files isn't guaranteed. Rewrote the test to assert the pagination contract (limit respected, cursor advances, follow-up pages keep returning results, paging surfaces more than one page of distinct results) instead of near-total coverage. (SQLite happens to be stable by insertion order, which is why the old assertion passed in CI but failed on Postgres.)seed_pagination_data.py— idempotent comment seeding with 1.05s timestamp spacing, so comment pagination is stable.test_pagination.py— event-seeding idempotency keyed on the summary, not the server-generated UID (prevents duplicate accumulation across runs).test_user_status.py— leak-free cleanup of the fresh test user via the admin client (the prior cleanup leaked it oncecreate_serverswapped the global client).Verification
CI is green on NC 32 + 33 (SQLite). Locally, against the PostgreSQL dev instances, the full suite previously ran
834 passed, 1 failedon both NC34 (RC1) and NC35 (master/dev) — the single failure being thistest_search_pagination. With the fix,test_search.pynow passes 17/17 on both NC34 and NC35 (re-run on Postgres), clearing that failure; ruff + pyright clean.(The
mailandsession_cachesuites are excluded locally — they need CI-specific SMTP / regular-password setup.)Summary by CodeRabbit
Documentation
Tests