From 23aa9b3230e0b9ca067130216a42a492c78d93b2 Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Fri, 18 Sep 2026 16:52:34 +0200 Subject: [PATCH 1/4] ci: gate PRs on unit tests only and move integration to a daily run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/ci.yml | 25 ++++++++++++++++++++ .github/workflows/run_tests.yml | 35 ++++++++++++++++------------ .github/workflows/scheduled_test.yml | 19 +++++++++++++++ DEVELOPMENT.md | 16 +++++++++++++ 4 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/scheduled_test.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 748ff652..f2b3367f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,11 @@ name: CI (unit) # SHA, and the duplicate legs raced each other against the Stream app several SDK repos # share. main and *.x are covered by release.yml, which runs this same reusable workflow # as the gate before tagging. +# +# Only the unit tests gate a pull request. The integration suite runs daily in +# scheduled_test.yml and again in release.yml before a tag, and is never a merge gate: +# it exercises a live app five SDK repos share, where another repo's run or a backend +# regression reddens it with nothing wrong in this one. on: pull_request: branches: [ "**" ] @@ -18,6 +23,26 @@ jobs: marker: 'not integration' secrets: inherit + # The one required status check on main. It carries no matrix, so it always publishes + # under this exact name: a matrix job skipped by `if:` publishes a single check run with + # the template unexpanded, which means per-leg contexts can never be satisfied on a + # Release PR. `always()` plus accepting `skipped` is what lets a Release PR through in + # seconds while a feature PR still has to go green. + tests-passed: + name: ๐Ÿงช Tests + needs: unit + if: always() + runs-on: ubuntu-latest + steps: + - name: Check the unit suite + env: + RESULT: ${{ needs.unit.result }} + run: | + case "$RESULT" in + success|skipped) echo "unit suite: $RESULT" ;; + *) echo "::error::unit suite reported $RESULT"; exit 1 ;; + esac + # Cancel in-flight runs for the same branch/PR when new commits arrive concurrency: group: ci-unit-${{ github.workflow }}-${{ github.ref }} diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index c1f9400a..30b45746 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -18,6 +18,12 @@ env: permissions: contents: read +# The caller decides what runs where. ci.yml passes `not integration`, so a pull request is +# gated on the unit tests alone. The `integration` marker reaches this workflow only from +# scheduled_test.yml and from release.yml's pre-tag gate, because the integration suite runs +# against a live Stream app that five SDK repos share: another repo's run, or a backend +# regression, can redden it with nothing wrong here, and that must never block a merge. +# # Every job below skips on a Release PR. release-please only bumps the version and rewrites # the changelog, and every source commit in one already passed this suite on the PR it came # from. The guard sits on each job rather than on the calling job in ci.yml, because a job @@ -25,17 +31,19 @@ permissions: # workflow that is never called produces no check at all. It tests the author as well as the # branch name: on its own, the name would let any PR, a fork's included, call its branch # release-please--x and skip every required check, which branch protection counts as met. -# github.actor is the third clause, and it is the pusher rather than the PR author, so a -# human commit pushed onto the Release PR to fix a conflict or a changelog entry is tested -# like any other commit instead of riding the skip into main untested. +# It deliberately leaves out github.actor. That is the pusher, and clicking Update branch +# attributes the resulting merge commit to whoever clicked, so keying on it would stop the +# skip firing on the normal release path. The cost is that a human commit pushed onto a +# Release PR is untested there; release.yml runs this suite on the merge commit before it +# tags. On a schedule or a push there is no pull request, every github.event.pull_request.* +# clause is empty, and the guard lets the job run. jobs: ruff: if: >- - ${{ !(github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' + ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' && github.event.pull_request.head.repo.full_name == github.repository && startsWith(github.head_ref, 'release-please--')) }} - name: Ruff + name: ๐Ÿงช Ruff runs-on: ubuntu-latest steps: - name: Checkout @@ -47,11 +55,10 @@ jobs: typecheck: if: >- - ${{ !(github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' + ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' && github.event.pull_request.head.repo.full_name == github.repository && startsWith(github.head_ref, 'release-please--')) }} - name: Type Check (ty) + name: ๐Ÿงช Type Check (ty) runs-on: ubuntu-latest steps: - name: Checkout @@ -66,11 +73,10 @@ jobs: # Video paths and manual test paths are defined in the Makefile. test-non-video: if: >- - ${{ !(github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' + ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' && github.event.pull_request.head.repo.full_name == github.repository && startsWith(github.head_ref, 'release-please--')) }} - name: Non-video tests (${{ matrix.python-version }}) + name: ๐Ÿงช Non-video tests (${{ matrix.python-version }}) environment: name: ci runs-on: ubuntu-latest @@ -97,11 +103,10 @@ jobs: # Uses STREAM_* credentials which have video enabled. test-video: if: >- - ${{ !(github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' + ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' && github.event.pull_request.head.repo.full_name == github.repository && startsWith(github.head_ref, 'release-please--')) }} - name: Video tests (${{ matrix.python-version }}) + name: ๐Ÿงช Video tests (${{ matrix.python-version }}) environment: name: ci runs-on: ubuntu-latest diff --git a/.github/workflows/scheduled_test.yml b/.github/workflows/scheduled_test.yml new file mode 100644 index 00000000..d657f943 --- /dev/null +++ b/.github/workflows/scheduled_test.yml @@ -0,0 +1,19 @@ +name: Scheduled tests +# The integration suite's home. It runs here every day and in release.yml before a tag, +# and nowhere else: it is advisory, never a merge gate. The suite is live against a +# Stream app that five SDK repos share, so a red run here is as often the app or the +# backend as this SDK, and that must not be able to block a pull request. +on: + schedule: + - cron: "0 9 * * *" + workflow_dispatch: + +permissions: + contents: read + +jobs: + integration: + uses: ./.github/workflows/run_tests.yml + with: + marker: 'integration' + secrets: inherit diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index d41ddd09..9eda9678 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -36,6 +36,22 @@ make test-jaeger # requires local Jaeger (docker run ... jaegertracing/all make test-prometheus # requires getstream[telemetry] deps ``` +### What CI runs + +| Trigger | What runs | Gates a merge? | +| --- | --- | --- | +| Pull request | ruff, ty, unit tests on five Python versions | yes, `๐Ÿงช Tests` is the required check | +| Daily at 09:00 UTC | the same jobs with `-m integration` | no | +| Push to `main` with a release pending | unit, then integration | no merge, but both gate the tag | + +Integration never gates a pull request. 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 here. Fix a red daily run, do not route around it. + +A Release PR skips the suite: every job in `run_tests.yml` is guarded, `๐Ÿงช Tests` reports +`skipped`, and branch protection accepts that. The merge commit is still tested in full +before it is tagged, so nothing untested reaches PyPI. + ### Linting and type checking ``` From 035502c0e3e48ccb8a71c307672dd24315184301 Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Fri, 18 Sep 2026 17:29:57 +0200 Subject: [PATCH 2/4] ci: split the integration lane into its own workflow and harden the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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". --- .github/workflows/ci.yml | 58 +++++++++++++++++++++------ .github/workflows/release.yml | 6 +-- .github/workflows/run_integration.yml | 53 ++++++++++++++++++++++++ .github/workflows/run_tests.yml | 52 +++++------------------- .github/workflows/scheduled_test.yml | 42 +++++++++++++++---- DEVELOPMENT.md | 20 +++++---- 6 files changed, 157 insertions(+), 74 deletions(-) create mode 100644 .github/workflows/run_integration.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2b3367f..c6bcfec5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,10 +4,8 @@ name: CI (unit) # share. main and *.x are covered by release.yml, which runs this same reusable workflow # as the gate before tagging. # -# Only the unit tests gate a pull request. The integration suite runs daily in -# scheduled_test.yml and again in release.yml before a tag, and is never a merge gate: -# it exercises a live app five SDK repos share, where another repo's run or a backend -# regression reddens it with nothing wrong in this one. +# Only the unit lane gates a pull request. `-m integration` lives in run_integration.yml, +# which the daily schedule and release.yml call and this workflow never does. on: pull_request: branches: [ "**" ] @@ -17,30 +15,64 @@ permissions: pull-requests: read jobs: + # Skipped on a Release PR: release-please only bumps the version and rewrites the + # changelog, and every source commit in one already passed this suite on the PR it came + # from. One guard here rather than one per job inside run_tests.yml, because `๐Ÿงช Tests` + # below publishes the required status check whether or not this job ran. + # + # The guard tests the author as well as the branch name: on its own the name would let + # any PR, a fork's included, call its branch release-please--x and skip the gate, which + # branch protection would count as met. It deliberately leaves out github.actor, which is + # the pusher: clicking Update branch attributes the merge commit to whoever clicked, so + # keying on it stops the skip firing on the normal release path. The cost is that a human + # commit pushed onto a Release PR is untested there, and release.yml runs the suite on the + # merge commit before it tags. unit: + if: >- + ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' + && github.event.pull_request.head.repo.full_name == github.repository + && startsWith(github.head_ref, 'release-please--')) }} uses: ./.github/workflows/run_tests.yml - with: - marker: 'not integration' secrets: inherit # The one required status check on main. It carries no matrix, so it always publishes # under this exact name: a matrix job skipped by `if:` publishes a single check run with - # the template unexpanded, which means per-leg contexts can never be satisfied on a - # Release PR. `always()` plus accepting `skipped` is what lets a Release PR through in - # seconds while a feature PR still has to go green. + # the template unexpanded, so per-leg contexts could never be satisfied on a Release PR. + # + # `skipped` is accepted only when the Release PR condition above actually holds, and the + # expression is repeated rather than shared because Actions has no way to share one. If + # the two ever drift, this job fails and the PR goes red, which is the safe direction: a + # skip nobody asked for is exactly what a required check exists to catch. tests-passed: name: ๐Ÿงช Tests needs: unit - if: always() + if: ${{ !cancelled() }} runs-on: ubuntu-latest steps: - - name: Check the unit suite + - name: Check the unit lane env: RESULT: ${{ needs.unit.result }} + RELEASE_PR: >- + ${{ github.event.pull_request.user.login == 'github-actions[bot]' + && github.event.pull_request.head.repo.full_name == github.repository + && startsWith(github.head_ref, 'release-please--') }} run: | case "$RESULT" in - success|skipped) echo "unit suite: $RESULT" ;; - *) echo "::error::unit suite reported $RESULT"; exit 1 ;; + success) + echo "unit lane passed" + ;; + skipped) + if [ "$RELEASE_PR" = "true" ]; then + echo "release pr: unit lane skipped by design" + else + echo "::error::the unit lane was skipped on a PR that is not a Release PR" + exit 1 + fi + ;; + *) + echo "::error::unit lane reported $RESULT" + exit 1 + ;; esac # Cancel in-flight runs for the same branch/PR when new commits arrive diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a1e65af3..c2f8908f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -164,17 +164,13 @@ jobs: needs: detect if: needs.detect.outputs.ready == 'true' uses: ./.github/workflows/run_tests.yml - with: - marker: 'not integration' secrets: inherit test-integration: name: Test (integration) needs: detect if: needs.detect.outputs.ready == 'true' - uses: ./.github/workflows/run_tests.yml - with: - marker: 'integration' + uses: ./.github/workflows/run_integration.yml secrets: inherit # Irreversible half. diff --git a/.github/workflows/run_integration.yml b/.github/workflows/run_integration.yml new file mode 100644 index 00000000..7949f262 --- /dev/null +++ b/.github/workflows/run_integration.yml @@ -0,0 +1,53 @@ +name: _run-integration +on: + workflow_call: + secrets: { } + +# Serialize every integration run on a branch instead of cancelling: the daily schedule and +# release.yml's pre-tag gate both target main, and `github.workflow` inside a reusable +# workflow resolves to the *caller's* name, so keying the group on it would put them in +# separate lanes and fire two suites at the shared Stream app at once. cancel-in-progress +# stays false so a scheduled run can never cancel the gate a release is waiting on. +concurrency: + group: run-integration-${{ github.ref }} + cancel-in-progress: false + +env: + UV_FROZEN: "1" + +permissions: + contents: read + +# The integration lane, and the only place `-m integration` runs. It is advisory: it runs +# daily and again before a tag, never as a status check on a pull request. The suite talks +# to a live Stream app that five SDK repos share, so another repo's run or a backend +# regression can redden it with nothing wrong here. +# +# Only the video half is here on purpose. Every `@pytest.mark.integration` in the repo is +# under tests/rtc or in the two *_manual.py files, and `make test` ignores both, so a +# non-video leg would collect nothing and the Makefile would report that as success. Add +# the leg back the day the chat and feeds suites carry the marker. +jobs: + integration-video: + name: ๐Ÿงช Video integration (${{ matrix.python-version }}) + environment: + name: ci + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v5 + - name: Install dependencies + uses: ./.github/actions/python-uv-setup + with: + python-version: ${{ matrix.python-version }} + - name: Run integration tests + env: + STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} + STREAM_API_SECRET: ${{ secrets.STREAM_API_SECRET }} + STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} + run: make test-video MARKER="integration" diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 30b45746..90f60188 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -1,14 +1,9 @@ name: _run-tests on: workflow_call: - inputs: - marker: - description: 'pytest -m expression (e.g., `not integration` or `integration`)' - required: true - type: string secrets: { } concurrency: - group: ${{ github.workflow }}-${{ github.ref }}-${{ inputs.marker }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true env: @@ -18,31 +13,16 @@ env: permissions: contents: read -# The caller decides what runs where. ci.yml passes `not integration`, so a pull request is -# gated on the unit tests alone. The `integration` marker reaches this workflow only from -# scheduled_test.yml and from release.yml's pre-tag gate, because the integration suite runs -# against a live Stream app that five SDK repos share: another repo's run, or a backend -# regression, can redden it with nothing wrong here, and that must never block a merge. +# The unit lane. Every job here runs offline-capable checks plus the chat, feeds and video +# suites with pytest's default marker expression, `not integration`. The `integration` +# marker lives in run_integration.yml, which only the daily schedule and release.yml's +# pre-tag gate call, so a live RTC failure can never block a merge. # -# Every job below skips on a Release PR. release-please only bumps the version and rewrites -# the changelog, and every source commit in one already passed this suite on the PR it came -# from. The guard sits on each job rather than on the calling job in ci.yml, because a job -# skipped by `if:` reports success and satisfies a required status check, while a reusable -# workflow that is never called produces no check at all. It tests the author as well as the -# branch name: on its own, the name would let any PR, a fork's included, call its branch -# release-please--x and skip every required check, which branch protection counts as met. -# It deliberately leaves out github.actor. That is the pusher, and clicking Update branch -# attributes the resulting merge commit to whoever clicked, so keying on it would stop the -# skip firing on the normal release path. The cost is that a human commit pushed onto a -# Release PR is untested there; release.yml runs this suite on the merge commit before it -# tags. On a schedule or a push there is no pull request, every github.event.pull_request.* -# clause is empty, and the guard lets the job run. +# There is no Release PR guard in this file. It sits on the calling job in ci.yml, which is +# the only caller that can see a pull request at all, and ci.yml's `๐Ÿงช Tests` job publishes +# the required status check whether or not this workflow ran. jobs: ruff: - if: >- - ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' - && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.head_ref, 'release-please--')) }} name: ๐Ÿงช Ruff runs-on: ubuntu-latest steps: @@ -54,10 +34,6 @@ jobs: run: make lint typecheck: - if: >- - ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' - && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.head_ref, 'release-please--')) }} name: ๐Ÿงช Type Check (ty) runs-on: ubuntu-latest steps: @@ -72,10 +48,6 @@ jobs: # Uses STREAM_CHAT_* credentials which do NOT have video enabled. # Video paths and manual test paths are defined in the Makefile. test-non-video: - if: >- - ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' - && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.head_ref, 'release-please--')) }} name: ๐Ÿงช Non-video tests (${{ matrix.python-version }}) environment: name: ci @@ -97,15 +69,11 @@ jobs: STREAM_API_KEY: ${{ vars.STREAM_CHAT_API_KEY }} STREAM_API_SECRET: ${{ secrets.STREAM_CHAT_API_SECRET }} STREAM_BASE_URL: ${{ vars.STREAM_CHAT_BASE_URL }} - run: make test MARKER="${{ inputs.marker }}" + run: make test # โ”€โ”€ Video tests (video-enabled credentials) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ # Uses STREAM_* credentials which have video enabled. test-video: - if: >- - ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' - && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.head_ref, 'release-please--')) }} name: ๐Ÿงช Video tests (${{ matrix.python-version }}) environment: name: ci @@ -127,4 +95,4 @@ jobs: STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} STREAM_API_SECRET: ${{ secrets.STREAM_API_SECRET }} STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} - run: make test-video MARKER="${{ inputs.marker }}" + run: make test-video diff --git a/.github/workflows/scheduled_test.yml b/.github/workflows/scheduled_test.yml index d657f943..df32d732 100644 --- a/.github/workflows/scheduled_test.yml +++ b/.github/workflows/scheduled_test.yml @@ -1,8 +1,8 @@ name: Scheduled tests -# The integration suite's home. It runs here every day and in release.yml before a tag, -# and nowhere else: it is advisory, never a merge gate. The suite is live against a -# Stream app that five SDK repos share, so a red run here is as often the app or the -# backend as this SDK, and that must not be able to block a pull request. +# The integration lane's home. It runs here every day and in release.yml before a tag, and +# nowhere else: integration is advisory and never a status check on a pull request. The +# suite talks to a live Stream app five SDK repos share, so a red run here is as often the +# app or the backend as this SDK, and that must not be able to block a merge. on: schedule: - cron: "0 9 * * *" @@ -13,7 +13,35 @@ permissions: jobs: integration: - uses: ./.github/workflows/run_tests.yml - with: - marker: 'integration' + uses: ./.github/workflows/run_integration.yml secrets: inherit + + # A daily run nobody is told about is the same as no daily run: a failure here lands on no + # PR, and GitHub emails scheduled-workflow failures only to whoever last edited the cron. + # One open issue per red streak, a comment per later failure, closed by hand once green. + report: + name: Report a red run + needs: integration + if: failure() + runs-on: ubuntu-latest + permissions: + issues: write + steps: + - name: Open or update the tracking issue + env: + GH_TOKEN: ${{ github.token }} + TITLE: Daily integration run is red + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + num="$(gh issue list --repo "$GITHUB_REPOSITORY" --state open --search "in:title \"$TITLE\"" \ + --json number,title --jq "map(select(.title == \"$TITLE\")) | .[0].number // empty")" + if [ -n "$num" ]; then + gh issue comment "$num" --repo "$GITHUB_REPOSITORY" --body "Still red: $RUN_URL" + else + gh issue create --repo "$GITHUB_REPOSITORY" --title "$TITLE" --body \ + "The daily \`-m integration\` run failed: $RUN_URL + + Integration is advisory, so nothing is blocked by this. It still has to be read: the suite + runs against a live Stream app shared by five SDK repos, so decide whether this is the app, + the backend, or this SDK, and close the issue once a daily run is green again." + fi diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 9eda9678..23230e15 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -40,17 +40,23 @@ make test-prometheus # requires getstream[telemetry] deps | Trigger | What runs | Gates a merge? | | --- | --- | --- | -| Pull request | ruff, ty, unit tests on five Python versions | yes, `๐Ÿงช Tests` is the required check | -| Daily at 09:00 UTC | the same jobs with `-m integration` | no | -| Push to `main` with a release pending | unit, then integration | no merge, but both gate the tag | +| Pull request | `run_tests.yml`: ruff, ty, and the non-video and video suites on five Python versions | yes, `๐Ÿงช Tests` is the required check | +| Daily at 09:00 UTC | `run_integration.yml`: the video suite with `-m integration` | no | +| Push to `main` with a release pending | both, in that order | no merge, but both gate the tag | Integration never gates a pull request. 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 here. Fix a red daily run, do not route around it. +wrong here. A red daily run opens an issue titled "Daily integration run is red"; fix it, +do not route around it. -A Release PR skips the suite: every job in `run_tests.yml` is guarded, `๐Ÿงช Tests` reports -`skipped`, and branch protection accepts that. The merge commit is still tested in full -before it is tagged, so nothing untested reaches PyPI. +Note what the `integration` marker means here. It is not "talks to the API": every +`@pytest.mark.integration` in the repo is under `tests/rtc/` or in the two `*_manual.py` +files, so it selects the WebRTC tests. The chat and feeds suites carry no marker and do +call the live API, which means the pull-request gate is not offline today. + +A Release PR skips the suite: `ci.yml`'s `unit` job is guarded, `๐Ÿงช Tests` passes in +seconds on a `skipped` unit result, and branch protection accepts that. The merge commit +is still tested in full before it is tagged, so nothing untested reaches PyPI. ### Linting and type checking From cd27a4aeb5a927f71d947c28e20496b63a2e380b Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Fri, 18 Sep 2026 17:47:40 +0200 Subject: [PATCH 3/4] test: mark every live-app test integration so the PR lane runs offline 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. --- .github/workflows/ci.yml | 1 - .github/workflows/release.yml | 8 ++++-- .github/workflows/run_integration.yml | 30 ++++++++++++++++++--- .github/workflows/run_tests.yml | 25 +++++++---------- DEVELOPMENT.md | 36 ++++++++++++++----------- tests/rtc/coordinator/test_connect.py | 5 ++++ tests/rtc/coordinator/test_heartbeat.py | 5 ++++ tests/rtc/test_join.py | 6 +++++ tests/rtc/test_video_properties.py | 5 ++++ tests/test_chat_channel.py | 6 +++++ tests/test_chat_draft.py | 5 ++++ tests/test_chat_integration.py | 18 +++++++++++-- tests/test_chat_message.py | 5 ++++ tests/test_chat_misc.py | 5 ++++ tests/test_chat_moderation.py | 6 +++++ tests/test_chat_polls.py | 6 +++++ tests/test_chat_reminders_locations.py | 5 ++++ tests/test_chat_team_usage_stats.py | 6 +++++ tests/test_chat_user.py | 6 +++++ tests/test_client.py | 2 ++ tests/test_feed_integration.py | 5 ++++ tests/test_video_examples.py | 5 ++++ tests/test_video_integration.py | 5 ++++ tests/test_video_openai.py | 5 ++++ 24 files changed, 170 insertions(+), 41 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6bcfec5..95fdabcc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,6 @@ jobs: && github.event.pull_request.head.repo.full_name == github.repository && startsWith(github.head_ref, 'release-please--')) }} uses: ./.github/workflows/run_tests.yml - secrets: inherit # The one required status check on main. It carries no matrix, so it always publishes # under this exact name: a matrix job skipped by `if:` publishes a single check run with diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c2f8908f..958efa83 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -164,8 +164,12 @@ jobs: needs: detect if: needs.detect.outputs.ready == 'true' uses: ./.github/workflows/run_tests.yml - secrets: inherit + # Advisory, exactly as on a pull request. It is deliberately absent from `release`'s + # `needs`: a flaky shared-app failure here would skip tagging, and the Release PR is + # already merged carrying `autorelease: pending`, so every later push would find a + # pending release at a different sha and stand down. Releases would stay wedged until + # someone re-ran one run or stripped the label by hand. The unit lane still gates. test-integration: name: Test (integration) needs: detect @@ -176,7 +180,7 @@ jobs: # Irreversible half. release: name: ๐Ÿš€ Tag and release - needs: [detect, test-unit, test-integration] + needs: [detect, test-unit] if: needs.detect.outputs.ready == 'true' runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/.github/workflows/run_integration.yml b/.github/workflows/run_integration.yml index 7949f262..40ec5e70 100644 --- a/.github/workflows/run_integration.yml +++ b/.github/workflows/run_integration.yml @@ -23,11 +23,33 @@ permissions: # to a live Stream app that five SDK repos share, so another repo's run or a backend # regression can redden it with nothing wrong here. # -# Only the video half is here on purpose. Every `@pytest.mark.integration` in the repo is -# under tests/rtc or in the two *_manual.py files, and `make test` ignores both, so a -# non-video leg would collect nothing and the Makefile would report that as success. Add -# the leg back the day the chat and feeds suites carry the marker. +# Both halves run here, on the two credential sets: the non-video leg uses the chat app and +# the video leg the video-enabled one, the same split the unit lane uses. jobs: + integration-non-video: + name: ๐Ÿงช Non-video integration (${{ matrix.python-version }}) + environment: + name: ci + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v5 + - name: Install dependencies + uses: ./.github/actions/python-uv-setup + with: + python-version: ${{ matrix.python-version }} + - name: Run integration tests + env: + STREAM_API_KEY: ${{ vars.STREAM_CHAT_API_KEY }} + STREAM_API_SECRET: ${{ secrets.STREAM_CHAT_API_SECRET }} + STREAM_BASE_URL: ${{ vars.STREAM_CHAT_BASE_URL }} + run: make test MARKER="integration" + integration-video: name: ๐Ÿงช Video integration (${{ matrix.python-version }}) environment: diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 90f60188..cbad9eab 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -13,10 +13,15 @@ env: permissions: contents: read -# The unit lane. Every job here runs offline-capable checks plus the chat, feeds and video -# suites with pytest's default marker expression, `not integration`. The `integration` -# marker lives in run_integration.yml, which only the daily schedule and release.yml's -# pre-tag gate call, so a live RTC failure can never block a merge. +# The unit lane, and it is offline. Every test that talks to a live Stream app carries +# `@pytest.mark.integration`, and pytest's default expression here is `not integration`, so +# these jobs declare no environment and receive no credentials. That is deliberate and it is +# load-bearing twice over: a pull request from a fork, which gets no secrets, still goes +# green, and a live test added without the marker fails loudly here instead of quietly +# passing on someone else's credentials. +# +# The `integration` marker lives in run_integration.yml, which only the daily schedule and +# release.yml call, so a shared-app failure can never block a merge. # # There is no Release PR guard in this file. It sits on the calling job in ci.yml, which is # the only caller that can see a pull request at all, and ci.yml's `๐Ÿงช Tests` job publishes @@ -49,8 +54,6 @@ jobs: # Video paths and manual test paths are defined in the Makefile. test-non-video: name: ๐Ÿงช Non-video tests (${{ matrix.python-version }}) - environment: - name: ci runs-on: ubuntu-latest strategy: fail-fast: false @@ -65,18 +68,12 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Run tests - env: - STREAM_API_KEY: ${{ vars.STREAM_CHAT_API_KEY }} - STREAM_API_SECRET: ${{ secrets.STREAM_CHAT_API_SECRET }} - STREAM_BASE_URL: ${{ vars.STREAM_CHAT_BASE_URL }} run: make test # โ”€โ”€ Video tests (video-enabled credentials) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ # Uses STREAM_* credentials which have video enabled. test-video: name: ๐Ÿงช Video tests (${{ matrix.python-version }}) - environment: - name: ci runs-on: ubuntu-latest strategy: fail-fast: false @@ -91,8 +88,4 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Run tests - env: - STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} - STREAM_API_SECRET: ${{ secrets.STREAM_API_SECRET }} - STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} run: make test-video diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 23230e15..6148c144 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -22,7 +22,9 @@ make test-all # both of the above ``` Non-video and video tests are split because they require different Stream credentials. -The `MARKER` variable defaults to `"not integration"`. Override it for integration tests: +The `MARKER` variable defaults to `"not integration"`, which is every test that does not +touch a live Stream app, so the default needs no credentials. Override it to run the ones +that do: ``` make test-integration # runs both groups with -m "integration" @@ -38,25 +40,27 @@ make test-prometheus # requires getstream[telemetry] deps ### What CI runs -| Trigger | What runs | Gates a merge? | +| Trigger | What runs | Gates anything? | | --- | --- | --- | -| Pull request | `run_tests.yml`: ruff, ty, and the non-video and video suites on five Python versions | yes, `๐Ÿงช Tests` is the required check | -| Daily at 09:00 UTC | `run_integration.yml`: the video suite with `-m integration` | no | -| Push to `main` with a release pending | both, in that order | no merge, but both gate the tag | - -Integration never gates a pull request. 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 here. A red daily run opens an issue titled "Daily integration run is red"; fix it, -do not route around it. - -Note what the `integration` marker means here. It is not "talks to the API": every -`@pytest.mark.integration` in the repo is under `tests/rtc/` or in the two `*_manual.py` -files, so it selects the WebRTC tests. The chat and feeds suites carry no marker and do -call the live API, which means the pull-request gate is not offline today. +| Pull request | `run_tests.yml`: ruff, ty, and `-m "not integration"` on five Python versions | yes, `๐Ÿงช Tests` is the required check | +| Daily at 09:00 UTC | `run_integration.yml`: `-m integration`, both credential sets | no | +| Push to `main` with a release pending | both; only the unit lane gates the tag | unit yes, integration no | + +`@pytest.mark.integration` means one thing here: the test talks to a live Stream app. The +unit lane therefore runs with **no credentials at all**, no `environment:` and no +`STREAM_*` secrets. Keep it that way. A pull request from a fork gets no secrets and still +goes green, and a live test added without the marker fails loudly in CI instead of quietly +passing on someone else's credentials. + +Integration gates nothing, anywhere. It runs against an app five SDK repos share, so +another repo's run or a backend regression can redden it with nothing wrong here, and a +red pre-tag run used to wedge every later release behind `autorelease: pending`. A red +daily run opens an issue titled "Daily integration run is red". Fix it, do not route +around it. A Release PR skips the suite: `ci.yml`'s `unit` job is guarded, `๐Ÿงช Tests` passes in seconds on a `skipped` unit result, and branch protection accepts that. The merge commit -is still tested in full before it is tagged, so nothing untested reaches PyPI. +still runs the unit lane before it is tagged. ### Linting and type checking diff --git a/tests/rtc/coordinator/test_connect.py b/tests/rtc/coordinator/test_connect.py index 3d2d156a..8389ae72 100644 --- a/tests/rtc/coordinator/test_connect.py +++ b/tests/rtc/coordinator/test_connect.py @@ -17,6 +17,11 @@ from tests.conftest import skip_on_rate_limit +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + @pytest.mark.asyncio async def test_simple_connection_debug(): """Simple test to debug websocket server handler signature.""" diff --git a/tests/rtc/coordinator/test_heartbeat.py b/tests/rtc/coordinator/test_heartbeat.py index 4c30aef5..c9959d66 100644 --- a/tests/rtc/coordinator/test_heartbeat.py +++ b/tests/rtc/coordinator/test_heartbeat.py @@ -17,6 +17,11 @@ from getstream import Stream +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + @pytest.mark.asyncio async def test_heartbeat_sent_periodically(client: Stream): """Test that heartbeat messages are sent at regular intervals.""" diff --git a/tests/rtc/test_join.py b/tests/rtc/test_join.py index cac66adc..ab978465 100644 --- a/tests/rtc/test_join.py +++ b/tests/rtc/test_join.py @@ -20,6 +20,12 @@ # Shared function for process setup and error handling + +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def run_process_with_stream_client( process_type: str, call_id: str, diff --git a/tests/rtc/test_video_properties.py b/tests/rtc/test_video_properties.py index 481785b1..89844704 100644 --- a/tests/rtc/test_video_properties.py +++ b/tests/rtc/test_video_properties.py @@ -11,6 +11,11 @@ from getstream.video.rtc.pb.stream.video.sfu.models.models_pb2 import TRACK_TYPE_VIDEO +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + @pytest.mark.asyncio async def test_detect_video_properties(): """Test that video properties are correctly detected from a video file.""" diff --git a/tests/test_chat_channel.py b/tests/test_chat_channel.py index 973f67ab..c6c4939e 100644 --- a/tests/test_chat_channel.py +++ b/tests/test_chat_channel.py @@ -17,10 +17,16 @@ UserRequest, ) from tests.base import wait_for_task +import pytest ASSETS_DIR = Path(__file__).parent / "assets" +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + class TestChannelCRUD: def test_create_channel(self, client: Stream, random_users): """Create a channel without specifying an ID (distinct channel).""" diff --git a/tests/test_chat_draft.py b/tests/test_chat_draft.py index c09f0ce3..a0760bdd 100644 --- a/tests/test_chat_draft.py +++ b/tests/test_chat_draft.py @@ -14,6 +14,11 @@ ) +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def _create_draft(channel, text, user_id, parent_id=None): """Create a draft via raw HTTP (endpoint is client-side-only, not in generated SDK).""" message = {"text": text, "user_id": user_id} diff --git a/tests/test_chat_integration.py b/tests/test_chat_integration.py index 1c1410f8..285c895b 100644 --- a/tests/test_chat_integration.py +++ b/tests/test_chat_integration.py @@ -12,6 +12,7 @@ import warnings +@pytest.mark.integration def test_upsert_users(client: Stream): users = {} user_id = str(uuid.uuid4()) @@ -22,11 +23,13 @@ def test_upsert_users(client: Stream): client.update_users(users=users) +@pytest.mark.integration def test_query_users(client: Stream): response = client.query_users(QueryUsersPayload(filter_conditions={})) assert response.data.users is not None +@pytest.mark.integration def test_update_users_partial(client: Stream): user_id = str(uuid.uuid4()) users = { @@ -50,6 +53,7 @@ def test_update_users_partial(client: Stream): assert response.data.users[user_id].custom["color"] == "blue" +@pytest.mark.integration def test_deactivate_and_reactivate_users(client: Stream): user_id = str(uuid.uuid4()) users = { @@ -66,6 +70,7 @@ def test_deactivate_and_reactivate_users(client: Stream): assert response.data.task_id is not None +@pytest.mark.integration def test_delete_user(client: Stream): user_id = str(uuid.uuid4()) users = { @@ -81,6 +86,7 @@ def test_delete_user(client: Stream): assert user_id not in user_ids +@pytest.mark.integration @pytest.mark.asyncio async def test_send_message(async_client: AsyncStream): channel = async_client.chat.channel("messaging", str(uuid.uuid4())) @@ -92,8 +98,16 @@ async def test_send_message(async_client: AsyncStream): ) -def test_from_env(): +def test_from_env(monkeypatch): + # Set the variables rather than reading whatever the environment happens to hold: this + # is the one test in the unit lane that touches STREAM_* config, and CI runs that lane + # with no credentials at all. + monkeypatch.setenv("STREAM_API_KEY", "key-from-env") + monkeypatch.setenv("STREAM_API_SECRET", "secret-from-env") + # Suppress the deprecation warning for this explicit compatibility check with warnings.catch_warnings(): warnings.simplefilter("ignore", category=DeprecationWarning) - Stream.from_env() + client = Stream.from_env() + + assert client.api_key == "key-from-env" diff --git a/tests/test_chat_message.py b/tests/test_chat_message.py index 7ebc80d1..a515db60 100644 --- a/tests/test_chat_message.py +++ b/tests/test_chat_message.py @@ -19,6 +19,11 @@ from tests.base import retry_on_transient_error +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def test_send_message(channel: Channel, random_user): """Send a message with skip_push option.""" response = channel.send_message( diff --git a/tests/test_chat_misc.py b/tests/test_chat_misc.py index bd0c93cf..a1906ed3 100644 --- a/tests/test_chat_misc.py +++ b/tests/test_chat_misc.py @@ -19,6 +19,11 @@ ) +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def test_get_app_settings(client: Stream): """Get application settings.""" response = client.get_app() diff --git a/tests/test_chat_moderation.py b/tests/test_chat_moderation.py index 87c31de7..22c12de9 100644 --- a/tests/test_chat_moderation.py +++ b/tests/test_chat_moderation.py @@ -10,6 +10,12 @@ QueryBannedUsersPayload, QueryMessageFlagsPayload, ) +import pytest + + +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration def test_ban_user(client: Stream, random_user, server_user): diff --git a/tests/test_chat_polls.py b/tests/test_chat_polls.py index c2333b8c..3648ef1d 100644 --- a/tests/test_chat_polls.py +++ b/tests/test_chat_polls.py @@ -8,6 +8,12 @@ PollOptionInput, VoteData, ) +import pytest + + +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration def test_create_get_update_delete_poll(client: Stream, random_user): diff --git a/tests/test_chat_reminders_locations.py b/tests/test_chat_reminders_locations.py index 33b0817c..51ca266f 100644 --- a/tests/test_chat_reminders_locations.py +++ b/tests/test_chat_reminders_locations.py @@ -10,6 +10,11 @@ from tests.base import retry_on_transient_error +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + class TestReminders: @pytest.fixture(autouse=True) def setup_channel_for_reminders(self, channel: Channel): diff --git a/tests/test_chat_team_usage_stats.py b/tests/test_chat_team_usage_stats.py index 8fd13f3a..5969286b 100644 --- a/tests/test_chat_team_usage_stats.py +++ b/tests/test_chat_team_usage_stats.py @@ -1,6 +1,12 @@ from datetime import date, timedelta from getstream import Stream +import pytest + + +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration def test_query_team_usage_stats_default(client: Stream): diff --git a/tests/test_chat_user.py b/tests/test_chat_user.py index e47b4a31..3d855600 100644 --- a/tests/test_chat_user.py +++ b/tests/test_chat_user.py @@ -14,6 +14,12 @@ UpdateUserPartialRequest, UserRequest, ) +import pytest + + +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration def test_upsert_users(client: Stream): diff --git a/tests/test_client.py b/tests/test_client.py index bcbfca31..5cd2c262 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -40,6 +40,7 @@ def test_incorrect_client_throws_exception(monkeypatch): Stream(api_key="xxx", api_secret="xxx", base_url="ftp://example.com") +@pytest.mark.integration def test_client_does_not_raise_exception_without_tracer(client: Stream, monkeypatch): # Monkey patch _get_tracer to always return None from getstream.common import telemetry @@ -50,6 +51,7 @@ def test_client_does_not_raise_exception_without_tracer(client: Stream, monkeypa assert response.data is not None +@pytest.mark.integration def test_client_works_with_no_otel(client: Stream, monkeypatch): # Monkey patch _get_tracer to always return None from getstream.common import telemetry diff --git a/tests/test_feed_integration.py b/tests/test_feed_integration.py index 9b95bbea..ab0ef3db 100644 --- a/tests/test_feed_integration.py +++ b/tests/test_feed_integration.py @@ -36,6 +36,11 @@ from getstream.stream_response import StreamResponse +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + class TestFeedIntegration: """ Systematic Integration tests for Feed operations diff --git a/tests/test_video_examples.py b/tests/test_video_examples.py index 6f7509e7..41437436 100644 --- a/tests/test_video_examples.py +++ b/tests/test_video_examples.py @@ -20,6 +20,11 @@ from tests.test_video_integration import get_openai_api_key_or_skip +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def test_setup_client(): from getstream import Stream diff --git a/tests/test_video_integration.py b/tests/test_video_integration.py index fbfc37d1..b4ebb40f 100644 --- a/tests/test_video_integration.py +++ b/tests/test_video_integration.py @@ -33,6 +33,11 @@ EXTERNAL_STORAGE_NAME = f"storage{uuid.uuid4()}" +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + def get_openai_api_key_or_skip(): """ Get the OpenAI API key from environment variables or skip the test. diff --git a/tests/test_video_openai.py b/tests/test_video_openai.py index e273e4d2..8c8508a4 100644 --- a/tests/test_video_openai.py +++ b/tests/test_video_openai.py @@ -29,6 +29,11 @@ ) +# Every test in this module talks to a live Stream app, so it is integration, not unit. +# The pull-request gate runs `-m "not integration"`; these run daily and before a release. +pytestmark = pytest.mark.integration + + class TestOpenAIPatching: """Tests for the OpenAI patching functionality.""" From 6d45c77f15522c355401d36411c04d7dca000428 Mon Sep 17 00:00:00 2001 From: Yun Wang Date: Fri, 18 Sep 2026 18:07:28 +0200 Subject: [PATCH 4/4] docs: cut the commentary back to what is not obvious from the code 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. --- .github/workflows/ci.yml | 33 ++++++++----------------- .github/workflows/release.yml | 8 +++--- .github/workflows/run_integration.yml | 17 ++++--------- .github/workflows/run_tests.yml | 16 +++--------- .github/workflows/scheduled_test.yml | 9 ++----- DEVELOPMENT.md | 23 ++++++++--------- tests/rtc/coordinator/test_connect.py | 2 -- tests/rtc/coordinator/test_heartbeat.py | 2 -- tests/rtc/test_join.py | 2 -- tests/rtc/test_video_properties.py | 2 -- tests/test_chat_channel.py | 2 -- tests/test_chat_draft.py | 2 -- tests/test_chat_integration.py | 4 +-- tests/test_chat_message.py | 2 -- tests/test_chat_misc.py | 2 -- tests/test_chat_moderation.py | 2 -- tests/test_chat_polls.py | 2 -- tests/test_chat_reminders_locations.py | 2 -- tests/test_chat_team_usage_stats.py | 2 -- tests/test_chat_user.py | 2 -- tests/test_feed_integration.py | 2 -- tests/test_video_examples.py | 2 -- tests/test_video_integration.py | 2 -- tests/test_video_openai.py | 2 -- 24 files changed, 34 insertions(+), 110 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 95fdabcc..841ea806 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,9 +3,6 @@ name: CI (unit) # SHA, and the duplicate legs raced each other against the Stream app several SDK repos # share. main and *.x are covered by release.yml, which runs this same reusable workflow # as the gate before tagging. -# -# Only the unit lane gates a pull request. `-m integration` lives in run_integration.yml, -# which the daily schedule and release.yml call and this workflow never does. on: pull_request: branches: [ "**" ] @@ -15,18 +12,11 @@ permissions: pull-requests: read jobs: - # Skipped on a Release PR: release-please only bumps the version and rewrites the - # changelog, and every source commit in one already passed this suite on the PR it came - # from. One guard here rather than one per job inside run_tests.yml, because `๐Ÿงช Tests` - # below publishes the required status check whether or not this job ran. - # - # The guard tests the author as well as the branch name: on its own the name would let - # any PR, a fork's included, call its branch release-please--x and skip the gate, which - # branch protection would count as met. It deliberately leaves out github.actor, which is - # the pusher: clicking Update branch attributes the merge commit to whoever clicked, so - # keying on it stops the skip firing on the normal release path. The cost is that a human - # commit pushed onto a Release PR is untested there, and release.yml runs the suite on the - # merge commit before it tags. + # Skipped on a Release PR: it only bumps the version and rewrites the changelog. Not + # keyed on github.actor, which is the pusher: clicking Update branch reattributes the + # merge commit to whoever clicked, and the skip would stop firing on the normal release + # path. A human commit pushed onto a Release PR is therefore untested until release.yml + # runs the lane on the merge commit. unit: if: >- ${{ !(github.event.pull_request.user.login == 'github-actions[bot]' @@ -34,14 +24,11 @@ jobs: && startsWith(github.head_ref, 'release-please--')) }} uses: ./.github/workflows/run_tests.yml - # The one required status check on main. It carries no matrix, so it always publishes - # under this exact name: a matrix job skipped by `if:` publishes a single check run with - # the template unexpanded, so per-leg contexts could never be satisfied on a Release PR. - # - # `skipped` is accepted only when the Release PR condition above actually holds, and the - # expression is repeated rather than shared because Actions has no way to share one. If - # the two ever drift, this job fails and the PR goes red, which is the safe direction: a - # skip nobody asked for is exactly what a required check exists to catch. + # The one required status check on main, and it carries no matrix on purpose: a matrix job + # skipped by `if:` publishes a single check run with the template unexpanded, so per-leg + # contexts could never be satisfied on a Release PR. `skipped` is accepted only when the + # condition above holds, repeated here because Actions cannot share an expression; if the + # two drift this fails, which is the safe direction. tests-passed: name: ๐Ÿงช Tests needs: unit diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 958efa83..f7654e96 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -165,11 +165,9 @@ jobs: if: needs.detect.outputs.ready == 'true' uses: ./.github/workflows/run_tests.yml - # Advisory, exactly as on a pull request. It is deliberately absent from `release`'s - # `needs`: a flaky shared-app failure here would skip tagging, and the Release PR is - # already merged carrying `autorelease: pending`, so every later push would find a - # pending release at a different sha and stand down. Releases would stay wedged until - # someone re-ran one run or stripped the label by hand. The unit lane still gates. + # Deliberately absent from `release`'s `needs`. A failure here would skip tagging while + # the Release PR is already merged carrying `autorelease: pending`, so every later push + # would stand down and releases would stay wedged until someone cleared it by hand. test-integration: name: Test (integration) needs: detect diff --git a/.github/workflows/run_integration.yml b/.github/workflows/run_integration.yml index 40ec5e70..a6ffcfa3 100644 --- a/.github/workflows/run_integration.yml +++ b/.github/workflows/run_integration.yml @@ -3,11 +3,9 @@ on: workflow_call: secrets: { } -# Serialize every integration run on a branch instead of cancelling: the daily schedule and -# release.yml's pre-tag gate both target main, and `github.workflow` inside a reusable -# workflow resolves to the *caller's* name, so keying the group on it would put them in -# separate lanes and fire two suites at the shared Stream app at once. cancel-in-progress -# stays false so a scheduled run can never cancel the gate a release is waiting on. +# A constant, not `github.workflow`, which inside a reusable workflow resolves to the +# caller's name: the daily run and the pre-tag run would land in separate lanes and hit the +# shared app at once. Queue rather than cancel, so the schedule cannot kill a release gate. concurrency: group: run-integration-${{ github.ref }} cancel-in-progress: false @@ -18,13 +16,8 @@ env: permissions: contents: read -# The integration lane, and the only place `-m integration` runs. It is advisory: it runs -# daily and again before a tag, never as a status check on a pull request. The suite talks -# to a live Stream app that five SDK repos share, so another repo's run or a backend -# regression can redden it with nothing wrong here. -# -# Both halves run here, on the two credential sets: the non-video leg uses the chat app and -# the video leg the video-enabled one, the same split the unit lane uses. +# The only place `-m integration` runs: daily and before a tag, never as a status check on a +# pull request. See DEVELOPMENT.md for why it gates nothing. jobs: integration-non-video: name: ๐Ÿงช Non-video integration (${{ matrix.python-version }}) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index cbad9eab..a10cb469 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -13,19 +13,9 @@ env: permissions: contents: read -# The unit lane, and it is offline. Every test that talks to a live Stream app carries -# `@pytest.mark.integration`, and pytest's default expression here is `not integration`, so -# these jobs declare no environment and receive no credentials. That is deliberate and it is -# load-bearing twice over: a pull request from a fork, which gets no secrets, still goes -# green, and a live test added without the marker fails loudly here instead of quietly -# passing on someone else's credentials. -# -# The `integration` marker lives in run_integration.yml, which only the daily schedule and -# release.yml call, so a shared-app failure can never block a merge. -# -# There is no Release PR guard in this file. It sits on the calling job in ci.yml, which is -# the only caller that can see a pull request at all, and ci.yml's `๐Ÿงช Tests` job publishes -# the required status check whether or not this workflow ran. +# The unit lane runs `-m "not integration"`, so it declares no environment and receives no +# credentials. Keep it that way: a fork PR gets no secrets and still goes green, and a live +# test added without the marker fails here instead of passing on someone else's. jobs: ruff: name: ๐Ÿงช Ruff diff --git a/.github/workflows/scheduled_test.yml b/.github/workflows/scheduled_test.yml index df32d732..5f7ba0e3 100644 --- a/.github/workflows/scheduled_test.yml +++ b/.github/workflows/scheduled_test.yml @@ -1,8 +1,5 @@ name: Scheduled tests -# The integration lane's home. It runs here every day and in release.yml before a tag, and -# nowhere else: integration is advisory and never a status check on a pull request. The -# suite talks to a live Stream app five SDK repos share, so a red run here is as often the -# app or the backend as this SDK, and that must not be able to block a merge. +# The integration lane's home. Advisory: see DEVELOPMENT.md. on: schedule: - cron: "0 9 * * *" @@ -16,9 +13,7 @@ jobs: uses: ./.github/workflows/run_integration.yml secrets: inherit - # A daily run nobody is told about is the same as no daily run: a failure here lands on no - # PR, and GitHub emails scheduled-workflow failures only to whoever last edited the cron. - # One open issue per red streak, a comment per later failure, closed by hand once green. + # A failure here lands on no PR, and GitHub emails it only to whoever last edited the cron. report: name: Report a red run needs: integration diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 6148c144..8544dcd0 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -46,21 +46,18 @@ make test-prometheus # requires getstream[telemetry] deps | Daily at 09:00 UTC | `run_integration.yml`: `-m integration`, both credential sets | no | | Push to `main` with a release pending | both; only the unit lane gates the tag | unit yes, integration no | -`@pytest.mark.integration` means one thing here: the test talks to a live Stream app. The -unit lane therefore runs with **no credentials at all**, no `environment:` and no -`STREAM_*` secrets. Keep it that way. A pull request from a fork gets no secrets and still -goes green, and a live test added without the marker fails loudly in CI instead of quietly -passing on someone else's credentials. +`@pytest.mark.integration` means one thing: the test talks to a live Stream app. The unit +lane therefore runs with no credentials, no `environment:` and no `STREAM_*`. Keep it that +way. A fork PR gets no secrets and still goes green, and a live test added without the +marker fails in CI instead of quietly passing on someone else's credentials. Integration gates nothing, anywhere. It runs against an app five SDK repos share, so -another repo's run or a backend regression can redden it with nothing wrong here, and a -red pre-tag run used to wedge every later release behind `autorelease: pending`. A red -daily run opens an issue titled "Daily integration run is red". Fix it, do not route -around it. - -A Release PR skips the suite: `ci.yml`'s `unit` job is guarded, `๐Ÿงช Tests` passes in -seconds on a `skipped` unit result, and branch protection accepts that. The merge commit -still runs the unit lane before it is tagged. +another repo's run or a backend regression can redden it with nothing wrong here, and a red +pre-tag run used to wedge every later release behind `autorelease: pending`. A red daily run +opens an issue titled "Daily integration run is red". Fix it, do not route around it. + +A Release PR skips the lane and `๐Ÿงช Tests` passes in seconds on a `skipped` result. The +merge commit still runs it before the tag. ### Linting and type checking diff --git a/tests/rtc/coordinator/test_connect.py b/tests/rtc/coordinator/test_connect.py index 8389ae72..461f1110 100644 --- a/tests/rtc/coordinator/test_connect.py +++ b/tests/rtc/coordinator/test_connect.py @@ -17,8 +17,6 @@ from tests.conftest import skip_on_rate_limit -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/rtc/coordinator/test_heartbeat.py b/tests/rtc/coordinator/test_heartbeat.py index c9959d66..0d6ec481 100644 --- a/tests/rtc/coordinator/test_heartbeat.py +++ b/tests/rtc/coordinator/test_heartbeat.py @@ -17,8 +17,6 @@ from getstream import Stream -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/rtc/test_join.py b/tests/rtc/test_join.py index ab978465..2ba03c60 100644 --- a/tests/rtc/test_join.py +++ b/tests/rtc/test_join.py @@ -21,8 +21,6 @@ # Shared function for process setup and error handling -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/rtc/test_video_properties.py b/tests/rtc/test_video_properties.py index 89844704..acdbcfdf 100644 --- a/tests/rtc/test_video_properties.py +++ b/tests/rtc/test_video_properties.py @@ -11,8 +11,6 @@ from getstream.video.rtc.pb.stream.video.sfu.models.models_pb2 import TRACK_TYPE_VIDEO -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_channel.py b/tests/test_chat_channel.py index c6c4939e..7b555e95 100644 --- a/tests/test_chat_channel.py +++ b/tests/test_chat_channel.py @@ -22,8 +22,6 @@ ASSETS_DIR = Path(__file__).parent / "assets" -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_draft.py b/tests/test_chat_draft.py index a0760bdd..24689f00 100644 --- a/tests/test_chat_draft.py +++ b/tests/test_chat_draft.py @@ -14,8 +14,6 @@ ) -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_integration.py b/tests/test_chat_integration.py index 285c895b..5ba24ee3 100644 --- a/tests/test_chat_integration.py +++ b/tests/test_chat_integration.py @@ -99,9 +99,7 @@ async def test_send_message(async_client: AsyncStream): def test_from_env(monkeypatch): - # Set the variables rather than reading whatever the environment happens to hold: this - # is the one test in the unit lane that touches STREAM_* config, and CI runs that lane - # with no credentials at all. + # Set them rather than reading the ambient environment: CI runs this lane with none. monkeypatch.setenv("STREAM_API_KEY", "key-from-env") monkeypatch.setenv("STREAM_API_SECRET", "secret-from-env") diff --git a/tests/test_chat_message.py b/tests/test_chat_message.py index a515db60..134f6076 100644 --- a/tests/test_chat_message.py +++ b/tests/test_chat_message.py @@ -19,8 +19,6 @@ from tests.base import retry_on_transient_error -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_misc.py b/tests/test_chat_misc.py index a1906ed3..ac01e4d3 100644 --- a/tests/test_chat_misc.py +++ b/tests/test_chat_misc.py @@ -19,8 +19,6 @@ ) -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_moderation.py b/tests/test_chat_moderation.py index 22c12de9..2227f30c 100644 --- a/tests/test_chat_moderation.py +++ b/tests/test_chat_moderation.py @@ -13,8 +13,6 @@ import pytest -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_polls.py b/tests/test_chat_polls.py index 3648ef1d..fa0b4f1b 100644 --- a/tests/test_chat_polls.py +++ b/tests/test_chat_polls.py @@ -11,8 +11,6 @@ import pytest -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_reminders_locations.py b/tests/test_chat_reminders_locations.py index 51ca266f..626877d2 100644 --- a/tests/test_chat_reminders_locations.py +++ b/tests/test_chat_reminders_locations.py @@ -10,8 +10,6 @@ from tests.base import retry_on_transient_error -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_team_usage_stats.py b/tests/test_chat_team_usage_stats.py index 5969286b..439c014b 100644 --- a/tests/test_chat_team_usage_stats.py +++ b/tests/test_chat_team_usage_stats.py @@ -4,8 +4,6 @@ import pytest -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_chat_user.py b/tests/test_chat_user.py index 3d855600..22b1f517 100644 --- a/tests/test_chat_user.py +++ b/tests/test_chat_user.py @@ -17,8 +17,6 @@ import pytest -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_feed_integration.py b/tests/test_feed_integration.py index ab0ef3db..f43efaf8 100644 --- a/tests/test_feed_integration.py +++ b/tests/test_feed_integration.py @@ -36,8 +36,6 @@ from getstream.stream_response import StreamResponse -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_video_examples.py b/tests/test_video_examples.py index 41437436..feb8b58c 100644 --- a/tests/test_video_examples.py +++ b/tests/test_video_examples.py @@ -20,8 +20,6 @@ from tests.test_video_integration import get_openai_api_key_or_skip -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_video_integration.py b/tests/test_video_integration.py index b4ebb40f..b32a2a83 100644 --- a/tests/test_video_integration.py +++ b/tests/test_video_integration.py @@ -33,8 +33,6 @@ EXTERNAL_STORAGE_NAME = f"storage{uuid.uuid4()}" -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration diff --git a/tests/test_video_openai.py b/tests/test_video_openai.py index 8c8508a4..4eb1674b 100644 --- a/tests/test_video_openai.py +++ b/tests/test_video_openai.py @@ -29,8 +29,6 @@ ) -# Every test in this module talks to a live Stream app, so it is integration, not unit. -# The pull-request gate runs `-m "not integration"`; these run daily and before a release. pytestmark = pytest.mark.integration