Fill in test hole and fix the bugs it exposed

Author: AArnott

src/NerdBank.GitVersioning.Tests/BuildIntegrationTests.cs

@@ -580,11 +580,15 @@ public async Task BuildNumber_SetInCI(VersionOptions versionOptions, IReadOnlyDi
 
     [Theory]
     [PairwiseData]
-    public async Task BuildNumber_VariousOptions(bool isPublic, VersionOptions.CloudBuildNumberCommitWhere where, VersionOptions.CloudBuildNumberCommitWhen when, [CombinatorialValues(0, 1, 2)] int extraBuildMetadataCount)
+    public async Task BuildNumber_VariousOptions(bool isPublic, VersionOptions.CloudBuildNumberCommitWhere where, VersionOptions.CloudBuildNumberCommitWhen when, [CombinatorialValues(0, 1, 2)] int extraBuildMetadataCount, [CombinatorialValues(1, 2)] int semVer)
     {
         var versionOptions = BuildNumberVersionOptionsBasis;
         versionOptions.CloudBuild.BuildNumber.IncludeCommitId.Where = where;
         versionOptions.CloudBuild.BuildNumber.IncludeCommitId.When = when;
+        versionOptions.NuGetPackageVersion = new VersionOptions.NuGetPackageVersionOptions
+        {
+            SemVer = semVer,
+        };
         this.WriteVersionFile(versionOptions);
         this.InitializeSourceControl();
 
@@ -841,6 +845,8 @@ private void AssertStandardProperties(VersionOptions versionOptions, BuildResult
             expectedBuildMetadata += "." + string.Join(".", additionalBuildMetadata);
         }
 
+        string expectedBuildMetadataWithoutCommitId = additionalBuildMetadata.Any() ? $"+{string.Join(".", additionalBuildMetadata)}" : string.Empty;
+
         Assert.Equal($"{version}", buildResult.AssemblyFileVersion);
         Assert.Equal($"{idAsVersion.Major}.{idAsVersion.Minor}.{idAsVersion.Build}{versionOptions.Version.Prerelease}{expectedBuildMetadata}", buildResult.AssemblyInformationalVersion);
 
@@ -861,10 +867,13 @@ private void AssertStandardProperties(VersionOptions versionOptions, BuildResult
         Assert.Equal(expectedBuildMetadata, buildResult.SemVerBuildSuffix);
 
         // NuGet is now SemVer 2.0 and will pass additional build metadata if provided
-        bool semVer1 = (versionOptions?.NuGetPackageVersion ?? VersionOptions.NuGetPackageVersionOptions.DefaultInstance).SemVer == 1;
-        string pkgVersionSuffix = buildResult.PublicRelease
-            ? (semVer1 ? string.Empty : expectedBuildMetadata)
-            : (semVer1 ? $"-g{commitIdShort}" : $"-g{commitIdShort}{(additionalBuildMetadata.Any() ? "+" : string.Empty)}{string.Join(".", additionalBuildMetadata)}");
+        bool semVer2 = (versionOptions?.NuGetPackageVersion ?? VersionOptions.NuGetPackageVersionOptions.DefaultInstance).SemVer == 2;
+        string pkgVersionSuffix = buildResult.PublicRelease ? string.Empty : $"-g{commitIdShort}";
+        if (semVer2)
+        {
+            pkgVersionSuffix += expectedBuildMetadataWithoutCommitId;
+        }
+
         Assert.Equal($"{idAsVersion.Major}.{idAsVersion.Minor}.{idAsVersion.Build}{GetSemVerAppropriatePrereleaseTag(versionOptions)}{pkgVersionSuffix}", buildResult.NuGetPackageVersion);
 
         var buildNumberOptions = versionOptions.CloudBuild?.BuildNumber ?? new VersionOptions.CloudBuildNumberOptions();

src/NerdBank.GitVersioning/VersionOracle.cs

@@ -276,20 +276,22 @@ public IDictionary<string, string> CloudBuildVersionVars
         /// </summary>
         /// <remarks>
         /// We always put the commit ID in the -prerelease tag for non-public releases.
-        /// But for public releases, we don't include it since it may be confusing for NuGet.
+        /// But for public releases, we don't include it in the +buildMetadata section since it may be confusing for NuGet.
         /// See https://github.com/AArnott/Nerdbank.GitVersioning/pull/132#issuecomment-307208561
         /// </remarks>
         private string SemVer2BuildMetadata =>
-            FormatBuildMetadata(this.PublicRelease ? this.BuildMetadata : this.BuildMetadataWithCommitId, true);
+            (this.PublicRelease ? string.Empty : this.GitCommitIdShortForNonPublicPrereleaseTag) + FormatBuildMetadata(this.BuildMetadata);
 
         private string PrereleaseVersionSemVer1 => MakePrereleaseSemVer1Compliant(this.PrereleaseVersion, SemVer1NumericIdentifierPadding);
 
+        private string GitCommitIdShortForNonPublicPrereleaseTag => (string.IsNullOrEmpty(this.PrereleaseVersion) ? "-" : ".") + $"g{this.GitCommitIdShort}";
+
         private VersionOptions.CloudBuildNumberOptions CloudBuildNumberOptions { get; }
 
         private int VersionHeightWithOffset => this.VersionHeight + this.VersionHeightOffset;
 
-        private static string FormatBuildMetadata(IEnumerable<string> identifiers, bool asSemVer2prerelease = false) =>
-            (identifiers?.Any() ?? false) ? (asSemVer2prerelease ? "." : "+") + string.Join(".", identifiers) : string.Empty;
+        private static string FormatBuildMetadata(IEnumerable<string> identifiers) =>
+            (identifiers?.Any() ?? false) ? "+" + string.Join(".", identifiers) : string.Empty;
 
         private static string FormatBuildMetadataSemVerV1(IEnumerable<string> identifiers) =>
             (identifiers?.Any() ?? false) ? "-" + string.Join("-", identifiers) : string.Empty;