ci: gate PRs on unit tests only and move integration to a daily run by mogita · Pull Request #292 · GetStream/stream-py

mogita · 2026-09-18T14:53:19Z

Ticket

Problem

A pull request has no required status check. main has strict: true with an empty required-checks list, so it forces the branch up to date and then requires nothing to be green.
The Release PR skip added in ci: skip the test suite on release-please Release PRs #291 never fires on a real release. It keys on github.actor, which is the pusher, and clicking Update branch attributes that merge commit to whoever clicked.
@pytest.mark.integration did not mean what the CI assumed. All nine of them were under tests/rtc/ or in a *_manual.py file, so the marker selected WebRTC, while the chat and feeds suites hit the live app carrying no marker. The pull-request gate was a live-app gate.
make test MARKER="integration" collected nothing and the Makefile turned pytest's exit 5 into success, so the non-video integration leg of the pre-tag gate had been passing without running a test.

Solution

The marker now means one thing: this test talks to a live Stream app.

17 modules get a module-level pytestmark, two mixed modules get per-test marks.
run_tests.yml is the unit lane and runs with no credentials: no environment:, no STREAM_*, and its callers pass no secrets. A fork PR goes green, and a live test added without the marker fails loudly instead of quietly passing on someone else's credentials.
run_integration.yml is new and is the only place -m integration runs, on both credential sets.
ci.yml adds tests-passed, named 🧪 Tests. No matrix, so it always publishes under that exact name: a matrix job skipped by if: publishes a single check run with the template unexpanded. It accepts skipped only when the Release PR condition actually holds, so a guard typo cannot publish it green with nothing run.
scheduled_test.yml is new: the integration lane daily at 09:00 UTC plus workflow_dispatch, and a job that opens or updates an issue when it fails.
release.yml drops test-integration from the release job's needs. A flaky failure there skipped tagging while the Release PR was already merged with autorelease: pending, so every later push found a pending release at a different sha and stood down. The unit lane still gates the tag.
The skip guard drops github.actor and moves to ci.yml's calling job, one copy instead of four.

Integration now gates nothing, anywhere. It runs against the app d2sj6pudbhz7 that five SDK repos share, so another repo's run or a backend regression reddens it with nothing wrong here.

How to verify

Measured locally with an empty .env, every STREAM_* variable unset and no network reachable:

make test        476 passed, 1 skipped, 209 deselected in 3.87s
make test-video  312 passed, 1 skipped,  94 deselected in 37.15s

788 tests still run on a pull request and none of them opens a socket. The non-video integration leg collects 209 tests where it collected no tests collected (686 deselected) before.

make lint && make typecheck both pass.
actionlint .github/workflows/*.yml. Two warnings about secrets.STREAM_*_API_SECRET remain in run_integration.yml, where those secrets really do come from the ci environment; run_tests.yml is clean now.
On the next Release PR: 🧪 Tests passes in seconds on a skipped unit result, including after Update branch.
gh workflow run "Scheduled tests" --repo GetStream/stream-py runs the integration lane on demand.

Review instructions

After this merges, 🧪 Tests has to be added to the required-checks list on main, which is empty today. Until then the check runs but gates nothing, so land and edit in the same sitting.

Summary by CodeRabbit

New Features
- Added daily integration test runs with automatic issue reporting when failures occur.
- Added separate unit and integration test lanes, including video coverage and multiple supported Python versions.
Bug Fixes
- Release tagging is no longer blocked by advisory integration-test failures.
- Test results now correctly distinguish expected skipped release checks from unexpected skips.
Documentation
- Clarified test categories, CI coverage, credentials requirements, and release gating behavior.

A pull request is now gated on one required check, 🧪 Tests, which covers ruff, ty and the unit suite on five Python versions. The integration marker no longer reaches ci.yml at all: it runs in the new scheduled_test.yml every day at 09:00 UTC and in release.yml before a tag, exactly as the CI/CD spec asks. Integration is advisory because it runs against a live Stream app that five SDK repos share, so another repo's run or a backend regression can redden it with nothing wrong in this one. Letting that block a merge is what the spec set out to avoid. The Release PR skip guard drops its github.actor clause. github.actor is the pusher, and clicking Update branch attributes the merge commit to whoever clicked, so keying on it stopped the skip firing on the normal release path. 🧪 Tests is a single job with no matrix, so it always publishes under that exact name. A matrix job skipped by `if:` publishes one check run with the template unexpanded, which is why the per-leg names could never work as required contexts.

coderabbitai · 2026-09-18T14:53:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9fd82af9-2db7-4f76-af94-3326758c0238

📥 Commits

Reviewing files that changed from the base of the PR and between 23aa9b3 and 6d45c77.

📒 Files selected for processing (25)

.github/workflows/ci.yml
.github/workflows/release.yml
.github/workflows/run_integration.yml
.github/workflows/run_tests.yml
.github/workflows/scheduled_test.yml
DEVELOPMENT.md
tests/rtc/coordinator/test_connect.py
tests/rtc/coordinator/test_heartbeat.py
tests/rtc/test_join.py
tests/rtc/test_video_properties.py
tests/test_chat_channel.py
tests/test_chat_draft.py
tests/test_chat_integration.py
tests/test_chat_message.py
tests/test_chat_misc.py
tests/test_chat_moderation.py
tests/test_chat_polls.py
tests/test_chat_reminders_locations.py
tests/test_chat_team_usage_stats.py
tests/test_chat_user.py
tests/test_client.py
tests/test_feed_integration.py
tests/test_video_examples.py
tests/test_video_integration.py
tests/test_video_openai.py

🚧 Files skipped from review as they are similar to previous changes (1)

DEVELOPMENT.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The pull request separates unit-test merge gating from integration testing. It marks live-app tests, adds a reusable integration workflow, schedules daily integration runs, and keeps Release PR tagging dependent only on unit tests.

Changes

CI test flow

Layer / File(s)	Summary
Required unit-test check `.github/workflows/ci.yml`, `.github/workflows/run_tests.yml`	The unit workflow runs without credentials. Release PRs can skip the unit job. `tests-passed` accepts that skip only for the matching Release PR condition and fails for other skipped or failed results.
Integration test classification `tests/rtc/`, `tests/test_.py`	Live-app tests receive the `integration` pytest marker. `test_from_env` now sets environment variables and verifies the resulting API key.
Scheduled integration tests `.github/workflows/run_integration.yml`, `.github/workflows/scheduled_test.yml`	A reusable workflow runs credentialed non-video and video integration tests on Python 3.10–3.14. A daily or manually dispatched workflow reports failed runs through the `Daily integration run is red` issue.
Release test separation `.github/workflows/release.yml`, `DEVELOPMENT.md`	Release runs invoke separate unit and integration workflows. Only the unit workflow remains in the release job requirements. The development guide documents the updated markers, triggers, and gating rules.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PullRequest
  participant CI
  participant run_tests
  participant tests-passed
  PullRequest->>CI: trigger pull request checks
  CI->>run_tests: run unit checks without credentials
  run_tests->>tests-passed: provide unit result
  tests-passed->>PullRequest: publish Tests status

sequenceDiagram
  participant Scheduler
  participant scheduled_test
  participant run_integration
  participant GitHubIssues
  Scheduler->>scheduled_test: trigger daily or manual run
  scheduled_test->>run_integration: invoke credentialed integration tests
  run_integration->>scheduled_test: return integration result
  scheduled_test->>GitHubIssues: report a failed run

Merge Risk: ⚪ Minimal · up to 6d45c

The separated unit and integration workflows have no identified actionable merge risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 19 files. (6 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: pull requests use unit tests as the merge gate, while integration tests move to a daily run.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 56.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 19 files. (6 skipped: 6 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Review found the daily run green without executing a test. Every @pytest.mark.integration in the repo is under tests/rtc or in a *_manual.py file, and `make test` ignores both, so `make test MARKER="integration"` collected nothing and the Makefile turned pytest's exit 5 into success. Measured on this tree: no tests collected (686 deselected) in 2.06s run_integration.yml now carries the video leg alone, which does collect, and run_tests.yml drops the marker input and is the unit lane only. Other review fixes: - The aggregator accepts `skipped` only when the Release PR condition actually holds, so a guard typo or a stray path filter fails the check instead of publishing it green with nothing run. - `if: always()` becomes `!cancelled()`. A cancelled run left `🧪 Tests` red on that SHA with only a re-run to clear it. - The Release PR guard moves to ci.yml's calling job. Four copies existed because a reusable workflow that is never called publishes no check, and the aggregator removed that constraint; it also kept pull_request expressions alive in the schedule and release callers, where they are dead weight. - run_integration.yml keys its concurrency group on a constant rather than github.workflow, which inside a reusable workflow is the caller's name. The daily run and the pre-tag run both target main and would otherwise hit the shared app at once. cancel-in-progress is false so the schedule cannot cancel the gate a release is waiting on. - The daily run opens or updates an issue when it fails. GitHub emails a scheduled failure only to whoever last edited the cron line. - ruff and ty no longer run in the daily job, so a lint failure on main cannot redden an integration signal. - DEVELOPMENT.md: `🧪 Tests` reports success on a Release PR, not skipped, and the `integration` marker means WebRTC here, not "talks to the API".

Review showed the marker did not mean what the CI design assumed. All nine @pytest.mark.integration in the repo were under tests/rtc or in a *_manual.py file, so it selected WebRTC, while the chat and feeds suites hit the live app carrying no marker at all. The pull-request gate was a live-app gate. Now the marker means one thing: the test talks to a live Stream app. 17 modules get a module-level pytestmark, and the two mixed modules get per-test marks. The counts, measured with an empty .env, no STREAM_* variables set and the base URL unreachable: make test 476 passed, 1 skipped, 209 deselected make test-video 312 passed, 1 skipped, 94 deselected 788 tests still run on a pull request and none of them opens a socket. The non-video integration leg now collects 209 tests where it collected none. Because the unit lane needs no credentials, run_tests.yml drops `environment: ci` and every STREAM_* variable, and its callers stop passing secrets. That is load-bearing twice: a fork PR, which gets no secrets, still goes green, and a live test added without the marker fails loudly instead of quietly passing on someone else's credentials. test_from_env now sets the variables it reads through monkeypatch and asserts the key lands, rather than depending on whatever the environment happens to hold. release.yml drops test-integration from the release job's needs. A flaky shared-app failure there would skip tagging while the Release PR was already merged with autorelease: pending, so every later push would find a pending release at a different sha and stand down, wedging releases until someone re-ran one run or stripped the label by hand. The unit lane still gates the tag.

98 comment lines in the diff for 194 of code. The same two sentences sat above all 17 module-level pytestmarks, and "the app five SDK repos share" was explained in three workflows and again in DEVELOPMENT.md. Kept the five facts that cost time to learn and that someone would otherwise undo: the unit lane has no credentials on purpose, a matrix job skipped by `if:` publishes one check with the template unexpanded, github.actor is absent because Update branch reattributes the commit, github.workflow inside a reusable workflow is the caller's name, and test-integration is deliberately out of release's needs. Everything else now lives once, in DEVELOPMENT.md. Down to 25 comment lines. No behaviour change: lint, format and the offline lane re-verified.

mogita deployed to ci September 18, 2026 14:53 — with GitHub Actions Active