ci: cross-platform test matrix (ubuntu, macos, windows × py3.11/3.12)#132
Merged
Conversation
Closes #131. The existing sdk-publish.yml only runs on v* tag push and doesn't run pytest at all, so regressions land on main with no machine-verified signal and platform-specific code paths (fcntl on POSIX, msvcrt on Windows) only ever run on one developer's local box. This workflow runs on push to main and pull_request to main. 6 jobs: 3 OSes × 2 Python versions, fail-fast disabled so a Windows-only bug doesn't hide a macOS-only one. Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
WalkthroughAdds a new GitHub Actions workflow Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 1-13: Add a top-level permissions block to this workflow so it
runs with least privilege: add permissions: contents: read at the workflow root
(alongside name, on, concurrency, jobs) so the actions/checkout step has the
required read-only access and the workflow does not inherit broader default
token scopes.
- Around line 41-43: The CI step named "Run pytest" currently executes all tests
(working-directory: Gradata, run: uv run pytest tests/ -q --tb=short); update
that run command to explicitly exclude integration tests by adding a pytest
marker filter (e.g., append -m "not integration") so tests marked with
`@pytest.mark.integration` are skipped in CI. Locate the "Run pytest" step and
modify its run string to include the marker exclusion.
- Around line 32-36: Replace the non-deterministic GitHub Actions input using
version: "latest" for the astral-sh/setup-uv@v4 step with a pinned, tested uv
release (e.g., "0.11.7"); update the workflow step that uses
astral-sh/setup-uv@v4 and change the version key from "latest" to the chosen
stable version string so CI runs are reproducible and won't break on future uv
releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4703316e-91c9-4694-a149-206bd9ebf7a0
📒 Files selected for processing (1)
.github/workflows/test.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
🔇 Additional comments (1)
.github/workflows/test.yml (1)
3-22: Matrix and trigger coverage matches the stated CI objective.This correctly sets up the 3×2 OS/Python matrix on
pushtomainandpull_requesttomain, withfail-fast: falsefor full signal.
- add workflow-level permissions: contents: read (least privilege) - pin astral-sh/setup-uv@v4 version from "latest" to "0.11.7" (reproducibility) - exclude integration tests in CI via -m "not integration" Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 17-19: The CI job named "test" can hang indefinitely; add a
timeout to the job configuration by setting the timeout-minutes property on the
"test" job (the job block whose name is "pytest ${{ matrix.os }} / py${{
matrix.python }}") to a reasonable limit (e.g., 10–30 minutes) so GitHub Actions
will cancel stuck runs; update the "test" job definition to include
timeout-minutes: <minutes> alongside runs-on and name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95ec12c8-c79b-4948-8be9-49c4a5920b00
📒 Files selected for processing (1)
.github/workflows/test.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
🔇 Additional comments (3)
.github/workflows/test.yml (3)
9-14: Good security baseline and sane concurrency control.Workflow-level least privilege (
contents: read) andcancel-in-progressare correctly set. This matches a test-only workflow and prevents stale duplicate runs.
20-24: Matrix configuration is correct and matches the PR objective.
ubuntu/macos/windows×3.11/3.12withfail-fast: falseis a solid cross-platform coverage setup.
44-46: Pytest invocation is aligned with repo test strategy.Running
tests/with-m "not integration"is consistent with the configuredintegrationmarker and keeps CI cost-safe.
Comment on lines
+17
to
+19
| test: | ||
| name: pytest ${{ matrix.os }} / py${{ matrix.python }} | ||
| runs-on: ${{ matrix.os }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a job timeout to avoid hung CI runners.
Consider setting timeout-minutes on the test job so intermittent hangs do not burn CI minutes indefinitely.
Suggested change
test:
name: pytest ${{ matrix.os }} / py${{ matrix.python }}
runs-on: ${{ matrix.os }}
+ timeout-minutes: 20
strategy:
fail-fast: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test: | |
| name: pytest ${{ matrix.os }} / py${{ matrix.python }} | |
| runs-on: ${{ matrix.os }} | |
| test: | |
| name: pytest ${{ matrix.os }} / py${{ matrix.python }} | |
| runs-on: ${{ matrix.os }} | |
| timeout-minutes: 20 | |
| strategy: | |
| fail-fast: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test.yml around lines 17 - 19, The CI job named "test" can
hang indefinitely; add a timeout to the job configuration by setting the
timeout-minutes property on the "test" job (the job block whose name is "pytest
${{ matrix.os }} / py${{ matrix.python }}") to a reasonable limit (e.g., 10–30
minutes) so GitHub Actions will cancel stuck runs; update the "test" job
definition to include timeout-minutes: <minutes> alongside runs-on and name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.github/workflows/test.ymlthat runspyteston every push to main and every PR.fail-fast: falseso one OS failing doesn't mask another.Why
sdk-publish.ymlonly runs on tag push and only builds — no tests. Before this PR, platform-specific code paths (fcntlvsmsvcrtinplatform_lock) were only verified on one developer's local machine. macOS parity was assumed.Test plan
Out of scope
Generated with Gradata