Split Unity and C# tests into separate jobs#3779
Conversation
|
This idea LGTM. If we're refactoring them anyway, I would probably make them both jobs under the existing CI workflow. |
| runs-on: spacetimedb-new-runner | ||
| container: | ||
| image: localhost:5000/spacetimedb-ci:latest | ||
| options: >- | ||
| --privileged | ||
| --cgroupns=host | ||
| timeout-minutes: 30 | ||
| env: | ||
| CARGO_TARGET_DIR: ${{ github.workspace }}/target | ||
| steps: | ||
| - name: Checkout repository | ||
| id: checkout-stdb | ||
| uses: actions/checkout@v4 | ||
|
|
||
| # Run cheap .NET tests first. If those fail, no need to run expensive Unity tests. | ||
|
|
||
| - name: Setup dotnet | ||
| uses: actions/setup-dotnet@v3 | ||
| with: | ||
| global-json-file: global.json | ||
|
|
||
| - name: Override NuGet packages | ||
| run: | | ||
| dotnet pack crates/bindings-csharp/BSATN.Runtime | ||
| dotnet pack crates/bindings-csharp/Runtime | ||
|
|
||
| # Write out the nuget config file to `nuget.config`. This causes the spacetimedb-csharp-sdk repository | ||
| # to be aware of the local versions of the `bindings-csharp` packages in SpacetimeDB, and use them if | ||
| # available. Otherwise, `spacetimedb-csharp-sdk` will use the NuGet versions of the packages. | ||
| # This means that (if version numbers match) we will test the local versions of the C# packages, even | ||
| # if they're not pushed to NuGet. | ||
| # See https://learn.microsoft.com/en-us/nuget/reference/nuget-config-file for more info on the config file. | ||
| cd sdks/csharp | ||
| ./tools~/write-nuget-config.sh ../.. | ||
|
|
||
| - name: Run .NET tests | ||
| working-directory: sdks/csharp | ||
| run: dotnet test -warnaserror | ||
|
|
||
| - name: Verify C# formatting | ||
| working-directory: sdks/csharp | ||
| run: dotnet format --no-restore --verify-no-changes SpacetimeDB.ClientSDK.sln | ||
|
|
||
| - name: Install Rust toolchain | ||
| uses: dtolnay/rust-toolchain@stable | ||
|
|
||
| - name: Cache Rust dependencies | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| workspaces: ${{ github.workspace }} | ||
| shared-key: spacetimedb | ||
| # Let the main CI job save the cache since it builds the most things | ||
| save-if: false | ||
|
|
||
| - name: Install SpacetimeDB CLI from the local checkout | ||
| run: | | ||
| cargo install --force --path crates/cli --locked --message-format=short | ||
| cargo install --force --path crates/standalone --locked --message-format=short | ||
| # Add a handy alias using the old binary name, so that we don't have to rewrite all scripts (incl. in submodules). | ||
| ln -sf $CARGO_HOME/bin/spacetimedb-cli $CARGO_HOME/bin/spacetime | ||
|
|
||
| - name: Check quickstart-chat bindings are up to date | ||
| working-directory: sdks/csharp | ||
| run: | | ||
| bash tools~/gen-quickstart.sh | ||
| "${GITHUB_WORKSPACE}"/tools/check-diff.sh examples~/quickstart-chat || { | ||
| echo 'Error: quickstart-chat bindings have changed. Please run `sdks/csharp/tools~/gen-quickstart.sh`.' | ||
| exit 1 | ||
| } | ||
|
|
||
| - name: Check client-api bindings are up to date | ||
| working-directory: sdks/csharp | ||
| run: | | ||
| bash tools~/gen-client-api.sh | ||
| "${GITHUB_WORKSPACE}"/tools/check-diff.sh src/SpacetimeDB/ClientApi || { | ||
| echo 'Error: Client API bindings are dirty. Please run `sdks/csharp/tools~/gen-client-api.sh`.' | ||
| exit 1 | ||
| } | ||
|
|
||
| - name: Start SpacetimeDB | ||
| run: | | ||
| spacetime start & | ||
| disown | ||
|
|
||
| - name: Run regression tests | ||
| run: | | ||
| bash sdks/csharp/tools~/run-regression-tests.sh | ||
| tools/check-diff.sh sdks/csharp/examples~/regression-tests || { | ||
| echo 'Error: Bindings are dirty. Please run `sdks/csharp/tools~/gen-regression-tests.sh`.' | ||
| exit 1 | ||
| } |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best way to fix the problem is to add an explicit permissions: block under the csharp-testsuite job in the .github/workflows/ci.yml workflow. As a minimal starting point, unless the job needs write-level permissions (for example, to upload reports to Checks or modify PRs), we should grant contents: read, which is sufficient for checking out code and accessing artifacts. To mirror the precedent set by the unity-testsuite job, you may optionally add additional permissions (e.g., checks: write) only if those capabilities are required by steps in the job (which may occur in test-reporting contexts). Otherwise, just set contents: read.
| @@ -655,6 +655,8 @@ | ||
| if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }} | ||
|
|
||
| csharp-testsuite: | ||
| permissions: | ||
| contents: read | ||
| runs-on: spacetimedb-new-runner | ||
| container: | ||
| image: localhost:5000/spacetimedb-ci:latest |
…ain permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: John Detter <4099508+jdetter@users.noreply.github.com>
…om/clockworklabs/SpacetimeDB into jdetter/split-unity-and-csharp-tests
e7281b6 to
3407ad9
Compare
bfops
left a comment
There was a problem hiding this comment.
Assuming all this code is just moved, this LGTM
Description of Changes
This has 2 benefits:
API and ABI breaking changes
None
Expected complexity level and risk
1
Testing