Skip to content

Allow for empty properties in dotnet msbuild#4165

Merged
devlead merged 1 commit into
cake-build:developfrom
Zulu-Inuoe:bugfix/empty-params-in-msbuild
Mar 8, 2026
Merged

Allow for empty properties in dotnet msbuild#4165
devlead merged 1 commit into
cake-build:developfrom
Zulu-Inuoe:bugfix/empty-params-in-msbuild

Conversation

@Zulu-Inuoe
Copy link
Copy Markdown
Contributor

Fixes #4158

@Zulu-Inuoe
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@augustoproiete augustoproiete self-requested a review June 6, 2023 04:24
Copy link
Copy Markdown
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Zulu-Inuoe

Overall looks good, but there's at least one test failing (which expects the old behavior). Could you take a look and amend the test(s) (or delete if doesn't make sense anymore)?

Actual:   (null)
  Stack Trace:
     at Cake.Common.Tests.Unit.Tools.DotNet.MSBuild.DotNetMSBuildBuilderTests.TheBuildMethod.Should_Throw_If_Property_Has_No_Value(String[] propertyValues) in D:\a\1\s\src\Cake.Common.Tests\Unit\Tools\DotNet\MSBuild\DotNetMSBuildBuilderTests.cs:line 204
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
  Failed Cake.Common.Tests.Unit.Tools.DotNet.MSBuild.DotNetMSBuildBuilderTests+TheBuildMethod.Should_Throw_If_Property_Has_No_Value(propertyValues: [""]) [< 1 ms]
  Error Message:
   Assert.IsType() Failure
Expected: System.ArgumentException

@augustoproiete
Copy link
Copy Markdown
Member

Hey @Zulu-Inuoe just following up on this one. Will you be able to get this to the finish line?

@devlead devlead force-pushed the bugfix/empty-params-in-msbuild branch from 5301bb4 to e4aa6f7 Compare March 8, 2026 16:48
Copy link
Copy Markdown
Member

@devlead devlead left a comment

Choose a reason for hiding this comment

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

Addressed the failing test.
LGTM 👍

@devlead devlead enabled auto-merge March 8, 2026 17:27
@devlead devlead disabled auto-merge March 8, 2026 17:27
@devlead devlead merged commit 013acf5 into cake-build:develop Mar 8, 2026
16 of 17 checks passed
@devlead
Copy link
Copy Markdown
Member

devlead commented Mar 8, 2026

@Zulu-Inuoe your changes have been merged, thanks for your contribution 👍

This was referenced May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot specify empty properties when using DotNetBuild

3 participants