ci: run integration tests against N, N-1, N-2 Dapr runtime versions - #1091
ashoksmore wants to merge 6 commits into
Conversation
Signed-off-by: Ashok More <ashoksmore7@gmail.com>
b689805 to
dbaabad
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1091 +/- ##
==========================================
- Coverage 82.78% 82.76% -0.02%
==========================================
Files 123 123
Lines 10077 10077
==========================================
- Hits 8342 8340 -2
- Misses 1735 1737 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sicoyle
left a comment
sicoyle
left a comment
There was a problem hiding this comment.
this is amazing - thank you!!! could you pls mv the python script into its own py file that we can then test and run locally more easily?
There was a problem hiding this comment.
Pull request overview
Updates the CI workflow to validate the Python SDK against the Dapr runtime compatibility window (N, N-1, N-2) by dynamically computing a runtime-version test matrix from the SDK version and dapr/dapr GitHub releases.
Changes:
- Adds a
preparejob that computes a compatibility matrix (Python 3.10–3.14 × runtime patch versions for N/N-1/N-2). - Updates
validateto consume the computed matrix and run integration + example test suites per runtime version. - Pins
setup-dapr-runtimeto an explicit runtime version per matrix entry (instead of always using “latest”).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| if [ ${{ github.event.client_payload.command }} = "ok-to-test" ]; then | ||
| echo "CHECKOUT_REPO=${{ github.event.client_payload.pull_head_repo }}" >> $GITHUB_ENV | ||
| echo "CHECKOUT_REF=${{ github.event.client_payload.pull_head_ref }}" >> $GITHUB_ENV | ||
| fi |
| run: | | ||
| CLI_VERSION=$(curl -fsS -H "Authorization: Bearer $GITHUB_TOKEN" \ | ||
| "https://api.github.com/repos/dapr/cli/releases?per_page=10" | \ | ||
| jq -r 'map(select(.prerelease == false)) | sort_by(.created_at) | reverse | .[0].tag_name | ltrimstr("v")') | ||
| if [ -z "$CLI_VERSION" ] || [ "$CLI_VERSION" = "null" ]; then | ||
| echo "Failed to resolve Dapr CLI version" && exit 1 | ||
| if [ ${{ github.event.client_payload.command }} = "ok-to-test" ]; then | ||
| echo "CHECKOUT_REPO=${{ github.event.client_payload.pull_head_repo }}" >> $GITHUB_ENV | ||
| echo "CHECKOUT_REF=${{ github.event.client_payload.pull_head_ref }}" >> $GITHUB_ENV | ||
| fi |
|
@ashoksmore Great idea! I was thinking this is potentially useful for other dapr repos too, so I think it'd a good idea to make it reusable. jobs:
versions:
runs-on: ubuntu-latest
outputs:
versions: ${{ steps.v.outputs.versions }} # ["1.18.2","1.17.5","1.16.8"]
steps:
- id: v
uses: dapr/.github/.github/actions/dapr-versions@main
test:
needs: versions
strategy:
matrix:
version: ${{ fromJson(needs.versions.outputs.versions) }}
steps:
- uses: dapr/.github/.github/actions/setup-dapr-cli@main
with: { version: ${{ matrix.version }} }What do you think? |
Signed-off-by: Ashok More <ashoksmore7@gmail.com>
|
@sicoyle Thanks for your review. I've addressed your feedback. Matrix logic is in tools/compute_compat_matrix.py with unit tests in tests/test_compute_compat_matrix.py. |
|
Hi @acroca, thanks for the suggestion on a shared dapr-versions action in dapr/.github. That makes a lot of sense for reuse across repos. I kept this PR focused on the python-sdk script per @sicoyle’s feedback, but I’d be happy to help on a follow-up PR in dapr/.github if that’s the direction you’d like to go. Let me know what you prefer. |
|
Hey @ashoksmore, I really like the initiative here, but I think there's a gap in the core approach we should sort out before it lands. The matrix runs the existing tests/integration and tests/examples suites unchanged against N, N-1 and N-2. The problem is those suites already include tests for APIs that only exist in recent runtimes, like conversation (the Llama setup step), jobs, mcp, workflow, crypto and distributed-lock. Running them against 1.17 (which is what N-2 resolves to today, with VERSION at 1.19.0.dev) they'll fail just because the feature didn't exist yet, not because anything regressed. Right now there's no mechanism in the repo to skip a test by runtime version, so the matrix would be red from day one. The thing we need to solve is making a red job mean something, so it tells us "we broke a supported runtime" and not "this feature is newer than that runtime". The way I read the N-2 guarantee is that existing behavior keeps working on older runtimes, not that new features work on them, but it'd be good to agree on that explicitly. My preference would be minimum runtime markers, something like One more thing worth putting on the table. This matrix tests the newest SDK against older runtimes. The reverse case, an older SDK against the latest runtime (for example someone on SDK 1.17 upgrading their dapr to 1.19), is probably the more common real world scenario, and it doesn't need any markers because the old tests only touch features that already existed. We could cover that by having the release-1.X branches run their suites against the latest runtime. Pinning release-1.17 to runtime 1.17 specifically isn't worth much, since that pair was already green at release. Not necessarily for this PR, but we should decide which directions we actually want to guarantee. This is also separate from the shared dapr/.github action idea from before, which is more about where the version resolution lives, so both can land. What do you think? |
|
@acroca Thanks for the detailed feedback; this helps a lot in understanding. You’re right. The matrix runs the full tests/integration/ and tests/examples/ suites unchanged, and the 1.17 jobs are failing on newer APIs, not because we regressed compatibility with older behavior. I had focused on the CI wiring from #1067 and didn’t account for the fact that many tests target features that didn’t exist on N-2 yet. I agree with your reading of the N/N-1/N-2 guarantee; existing behavior should keep working on supported older runtimes, not that every new feature must work on them. Without runtime-aware skipping, a red matrix job doesn’t mean much, it mostly says “this test needs a newer runtime.” I like the @pytest.mark.min_runtime("x.y") approach with a conftest hook (env var from the workflow or metadata lookup). That keeps the suite self-describing and avoids maintaining a separate “core” test list. Question for you: would you prefer the marker + conftest work in this PR before we enable the matrix, or land the matrix wiring first and follow up with markers in a separate PR? On the reverse-compat case (older SDK on latest runtime via release branches), that makes sense as a separate decision; happy to help there once we align on direction. On the shared dapr-versions action in dapr/.github; still happy to contribute a follow-up there too. Thanks again! |
Without those, this PR will never get green. We need to get a green build in order to approve the change, so it will have to be added in this PR. But before adding it, I would like to hear from the other @dapr/maintainers-python-sdk (and @dapr/approvers-python-sdk). |
|
yes, pls add the markers + conftest work into this PR to support it fully! Thanks for catching that @acroca and for being flexible @ashoksmore!! I am ok with this change applying to Python SDK first and then once we figure out things like the conftest/markers stuff and have it running successfully (IE we don't think of anything else necessary to roll this out to the rest of the SDKs), then we can generalize this work into the dpar/.github repo as a secondary followup task and come back and update here accordingly. And yes, would love for you @ashoksmore to lead that effort! That would have such a valuable impact on ALLLLLL of the Dapr SDKs which would be amazing! |
@acroca could you please scope out an issue for this? Even for this work, I am happy for us to use Python SDK as the "guinea pig" if you will here to test things out and see the things we need to account for before applying to the rest of the SDKs :) |
|
Hey @ashoksmore, will you willing to work on this new decorators approach? |
|
Yes, happy to take this on! I’ll add the @pytest.mark.min_runtime markers + conftest hooks in this PR and aim to push an update shortly. Will ping when it’s ready for another look. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
CasperGN
left a comment
CasperGN
left a comment
There was a problem hiding this comment.
Comment (a nudge, not a new review; @sicoyle's request-changes still stands)
@ashoksmore, checking in on the min_runtime markers you agreed to on 07-28. Nothing has been pushed since 06-22, and the stale bot closes this around 10-03. If you're still on it, a quick status note will keep it open. If you've run out of time, say so and one of us can take it over. For whoever picks it up, this is what's left:
- Blocking: the decorators approach isn't in the PR yet. There's no
min_runtimemarker inpyproject.tomland no conftest hook. Every run so far (06-13, 06-30, 07-13) has 5/51.17.10jobs red, e.g. job 86845282462: 6test_jobs*failures on 1.17. The examples step never runs after that, so we don't yet know how much oftests/examples/needs a marker too. Thedapr_headmarker that main added after your last merge (pyproject.toml on main) is the pattern to follow. Also, head14a04a7has no run-tests run at all, so it needs a fresh push or approval. - Blocking: N is silently dropped.
VERSIONis1.19.0.devand dapr has no 1.19 release or RC, so the skip-with-warning path turns the matrix into 1.18 + 1.17 only (the 07-13 prepare log says "no Dapr runtime release found for 1.19, skipping"). On main that means we test N-1/N-2 and drop the 1.18.0 floor main has today. Either treat N as "latest released minor" when the SDK minor is unreleased, or fail when fewer than 3 minors resolve (Copilot's thread, still open). - Blocking: the new
preparejob putsclient_payload.*straight into the shell (L38-L43). Main has since movedvalidateto theenv:+ quoted form.prepareshould do the same. Also bumpcheckout@v6to@v7(L46). - Non-blocking:
?per_page=100has no pagination. That page currently reaches back only to v1.16.0 (2025-09) and holds 2 entries for 1.15, so the "works unchanged on release branches" claim won't hold forrelease-1.17and older. - Non-blocking: cost goes from 5 to 15 jobs, each with its own Llama pull. Running N-1/N-2 on one Python version (3.10 is enough) and keeping the full Python range for N would give 7. The Python list is also now hardcoded in the script as well as in
build-tag.yaml, so the two can drift.
Your 06-22 commit covered @sicoyle's ask: the script is in tools/ and has unit tests (13 tests, all pass locally).
What I checked: all threads, the diff at 14a04a7, a trial merge onto current main (clean, only behind), the three run-tests runs and their job logs, the script's unit tests, and the script run against the live releases API. I did not run the integration matrix.
Description
Updates
run-tests.yamlto validate SDK compatibility against Dapr runtime versions N, N-1, and N-2 per the SDK compatibility policy.Changes:
preparejob that reads the SDKVERSIONand derives three runtime minorsdapr/daprreleasesvalidatejob matrix to run existing integration and example tests for each runtime × Python version (3.10-3.14)versiontosetup-dapr-runtimeinstead of always using the latest runtimeThe same workflow file should work on backported release branches without modification.
Issue reference
Fixes #1067
Checklist
build.yamllint/unit checks applytests/integration/andtests/examples/suites across runtime matrix (no new test files; coverage is via CI matrix)