-
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
Fix C# module smoketests #1475
Fix C# module smoketests #1475
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.
Gets the job done!
Oh wait, The PR in description fails with this error -
Is that means, error is specific to those PRs? |
…t version bump" This reverts commit c2868b3.
spacetime generate
creating broken C# projects@@ -17,6 +17,7 @@ def test_build_csharp_module(self): | |||
|
|||
try: | |||
|
|||
run_cmd("dotnet", "nuget", "locals", "all", "--clear", cwd=bindings, capture_stderr=True) |
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.
clear package cache. this isn't required to make the CI work again, but there did seem to be some kind of caching happening because the smoketests weren't passing or failing consistently. This hopefully makes them more consistent.
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.
(we think this also resolves a hypothetical issue where multiple open PRs make non-backwards-compatible changes to the NuGet packages)
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.
This feels like a good mitigation to fix the master. Thanks for working on this.
But I think there is still a need to have a general solution so that we don't run into the problem in future (not to be done by you or under the scope of this PR).
The general solution is to bump the package versions when there are breaking changes |
Co-authored-by: Zeke Foppa <github.com/bfops>
Description of Changes
#1440 restructured our NuGet packages, and correspondingly updated the
.csproj
files we generate. However, since we haven't actually published the restructured NuGet packages,.csproj
s expecting the new NuGet package structure will be broken, including the smoketests.It seems like this behavior was sometimes causing smoketests to fail. It's unclear why it's only sometimes, but our best guess is "something something caching", so this PR forcibly clears the NuGet package cache.
Additionally, we bump the versions of the local NuGet packages to 0.11, so that they cannot be found on the NuGet servers. This means that the smoketests can only find them in local directories, so they do not get confused about where to get these packages.
API and ABI breaking changes
No
Testing
spacetime init --lang csharp
followed byspacetime build
works successfully