Skip to content

Tighten package sources in C# smoketests #1735

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 28, 2024
Merged

Tighten package sources in C# smoketests #1735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71e900e
Tighten package sources in C# smoketests
RReverser Sep 24, 2024
c7c0807
Fallback to NuGet for C# SDK tests
RReverser Sep 28, 2024
d05121c
Fix generated XML
RReverser Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,18 @@ jobs:
<packageSources>
<!-- Local NuGet repositories -->
<add key="Local SpacetimeDB.BSATN.Runtime" value="../crates/bindings-csharp/BSATN.Runtime/bin/Release" />
<!-- Official NuGet.org server -->
<add key="NuGet.org" value="https://api.nuget.org/v3/index.json" />
</packageSources>
<packageSourceMapping>
<!-- Ensure that SpacetimeDB.BSATN.Runtime is used from the local folder. -->
<!-- Otherwise we risk an outdated version being quietly pulled from NuGet for testing. -->
<packageSource key="Local SpacetimeDB.BSATN.Runtime">
<package pattern="SpacetimeDB.BSATN.Runtime" />
</packageSource>
<!-- Fallback to NuGet for other packages. -->
<packageSource key="nuget.org">
<package pattern="*" />
</packageSource>
</packageSourceMapping>
Copy link
Collaborator

@bfops bfops Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

FWIW, the previous nuget-fallback behavior was entirely intentional (see the PR description that introduced it: #1503).

tl;dr it was set up so that, when we released breaking changes in this repo, the SDK tests would continue to use a package they were compatible with, rather than breaking for every future SpacetimeDB PR until someone fixed the C# SDK.

Copy link
Collaborator

@bfops bfops Sep 25, 2024

Choose a reason for hiding this comment

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

This way, the C# SDK tests only break if you fail to bump the version numbers when you make a breaking change.

I don't feel strongly about keeping it that way (and I believe it's not a required check, so this doesn't "deadlock" us), but that was the original intent.

Copy link
Contributor Author

@RReverser RReverser Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This way, the C# SDK tests only break if you fail to bump the version numbers when you make a breaking change.

Problem is, we also have the opposite sometimes - we bumped SpacetimeDB version, made some breaking changes, but C# SDK tests are still passing "successfully" because they are still pointing to old BSATN.* packages.

This is too easy to miss at the moment, so I do think the noisy error would be better in this context as it would allow us to catch issues sooner rather than later.

Copy link
Contributor Author

@RReverser RReverser Sep 25, 2024

Choose a reason for hiding this comment

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

Although I can see the arguments for the original behaviour too...

Copy link
Collaborator

@bfops bfops Sep 25, 2024

Choose a reason for hiding this comment

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

Sounds good - we should probably add a requirement that people breaking a downstream repo (in this case, the C# SDK) have a fix PR ready to go before merging their breaking change in this repo.

Copy link
Contributor Author

@RReverser RReverser Sep 25, 2024

Choose a reason for hiding this comment

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

I'd say let's try this stricter mode to catch breaking behaviour sooner; if it does prove too annoying, we can always revert those changes.

</configuration>
EOF

Expand Down
31 changes: 20 additions & 11 deletions smoketests/tests/csharp_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pathlib import Path
import shutil
import subprocess
import xml.etree.ElementTree as xml


@requires_dotnet
Expand All @@ -26,20 +27,28 @@ def test_build_csharp_module(self):

packed_projects = ["BSATN.Runtime", "Runtime"]

config = []
config.append("<?xml version=\"1.0\" encoding=\"utf-8\"?>")
config.append("<configuration>")
config.append("<packageSources>")
config.append("<!-- Local NuGet repositories -->")
config = xml.Element("configuration")

sources = xml.SubElement(config, "packageSources")
mappings = xml.SubElement(config, "packageSourceMapping")

for project in packed_projects:
# Add local build directories as NuGet repositories.
path = bindings / project / "bin" / "Release"
config.append("<add key=\"Local %s\" value=\"%s\" />\n" % (project, str(path)))
config.append("<!-- Official NuGet.org server -->")
config.append("<add key=\"NuGet.org\" value=\"https://api.nuget.org/v3/index.json\" />")
config.append("</packageSources>")
config.append("</configuration>")
project = f"SpacetimeDB.{project}"
xml.SubElement(sources, "add", key=project, value=str(path))

# Add strict package source mappings to ensure that
# SpacetimeDB.* packages are used from those directories
# and never from nuget.org.
#
# This prevents bugs where we silently used an outdated
# version which led to tests passing when they shouldn't.
mapping = xml.SubElement(mappings, "packageSource", key=project)
xml.SubElement(mapping, "package", pattern=project)

config = "\n".join(config)
xml.indent(config)
config = xml.tostring(config, encoding="unicode", xml_declaration=True)

print("Writing `nuget.config` contents:")
print(config)
Expand Down
Loading