Skip to content

Conversation

@jdetter
Copy link
Collaborator

@jdetter jdetter commented Nov 26, 2025

Description of Changes

This has 2 benefits:

  1. If the Unity test fails because of a license issue then we don't have to re-run the C# tests again as part of this flow. Re-running the Unity tests will be much faster if that's the only thing the job is doing.
  2. These tests will run faster because they will now run in parallel as separate CI jobs.

API and ABI breaking changes

None

Expected complexity level and risk

1

Testing

  • Both tests pass

@bfops
Copy link
Collaborator

bfops commented Nov 26, 2025

This idea LGTM. If we're refactoring them anyway, I would probably make them both jobs under the existing CI workflow.

@jdetter jdetter marked this pull request as ready for review November 26, 2025 19:48
Comment on lines +653 to +743
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

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI about 1 month 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.

Suggested changeset 1
.github/workflows/ci.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -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
EOF
@@ -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
Copilot is powered by AI and may make mistakes. Always verify output.
jdetter and others added 3 commits November 26, 2025 13:49
…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
@jdetter jdetter force-pushed the jdetter/split-unity-and-csharp-tests branch from e7281b6 to 3407ad9 Compare November 26, 2025 19:58
Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming all this code is just moved, this LGTM

@bfops bfops added this pull request to the merge queue Nov 26, 2025
Merged via the queue into master with commit 1d34217 Nov 26, 2025
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants