-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI - SpacetimeDB PRs run the C# SDK tests #1503
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comment nit but this looks great, thanks zeke 🙏
- name: Create /stdb dir | ||
run: | | ||
sudo mkdir /stdb | ||
sudo chmod 777 /stdb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌶️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good ol' copy 🍝
( cd crates/bindings-csharp/BSATN.Runtime && dotnet pack ) | ||
cd spacetimedb-csharp-sdk | ||
|
||
cat >nuget.config <<EOF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we throw a comment on this explaining what we're doing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added one - how does this look?
(did a final round of testing by manually downgrading the local package versions to match the NuGet versions, see the |
Description of Changes
Added a CI step that checks out
spacetimedb-csharp-sdk
and runs its test suite.In the current PR incarnation, if the local major/minor package versions are bumped, the C# SDK tests will continue to use the old ones. This is because the C# SDK repo will still have its version numbers pointed to e.g.
0.10.*
rather than e.g.0.11.*
. Is this a bug or a feature?I kind of think that the current behavior is correct - if we bump the major/minor version numbers, we're explicitly aware that we're releasing a breaking change. This effectively tests the experience of a current user of the C# SDK (on
master
). In that case, we can make this a required check.On the other hand, if we decide we want the C# SDK to always use the local versions, then this should not be made a required check. We may sometimes release breaking changes, which will need to happen in this repository before they happen in that one.
API and ABI breaking changes
No
Expected complexity level and risk
2
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!
dotnet test
is, in fact, runningbindings-csharp
versions to 0.10.1 (to match the latest version on NuGet), the tests use the local versions (and fail, because the C# SDK repo is not yet updated for the latest packages)