Fixes #72 (#74)

* Formatting of pre-release numbers for CI builds should ALWAYS include the pre-release number and fix for a pre-release build.
* Moved non-test functionality into Support namespace to help clarify role
* Clarified docs on behavior of CI version
    - Clearly that's a complex and subtle  point!

Author: smaillet

docfx/build-tasks/UnderstandingCIBuilds.md

@@ -32,6 +32,14 @@ Hopefully examples will make things more clear:
     * As with the previous example this is ordered AFTER the release it is based on
       and BEFORE the Patch+1 version (`v5.0.4-beta.0.1`).
 
+>[!NOTE]
+> As [[BUG] - CI version string formatting does not ALWAYS include pre-release information for a pre-release](https://github.com/UbiquityDotNET/CSemVer.GitBuild/issues/72)
+> points out the formatting of pre-release version numbers is different in CSemVer-CI. In a CI
+> version the `Number` and `Fix` values are ALWAYS included, even if `0`. In a CSemVer
+> things are more complex. In such a case, the `Number` and `Fix` are NOT shown if 0.
+> ***Unless*** the `Number` is `0` AND `Fix > 0` in that case the `Number` is shown
+> as a zero and the `Fix` is shown as it's non-zero value.
+
 ## lifetime scope of a CI Build
 The lifetime of a CI build is generally very short and once a version is released
 all CIs that led up to that release are essentially moot.
@@ -99,18 +107,18 @@ on the ordered integral form of the version and increments that, until it reache
 maximum)
 
 ### Ordered Version
-Ordered versions are a concept unique to Constrained Semantic versions. The constraints
-applied to a SemVer allow creation of an integral value for all versions, except CI
-builds. Ignoring CI builds for the moment, the ordered number is computed from the
-values of the various parts of a version as they are constrained by the CSemVer spec.
-The math involved is not important for this discussion. Just that each Constrained
-Version is representable as a distinct integral value (63 bits actually). A CSemVer-CI
-build has two elements the base build and the additional 'BuildIndex' and 'BuildName'
-components. This means the string, File version and ordered version numbers are
-confusingly different for a CI build. The ordered version number does NOT account for
-CI in any way. It is ONLY able to convert to/from a CSemVer. Thus, a CSemVer-CI has
-an ambiguous conversion. Should it convert the Patch+1 form in a string or the
-base build number?.
+Ordered versions are a concept unique to Constrained Semantic versions. The
+constraints applied to a SemVer allow creation of an integral value for all versions,
+except CI builds. Ignoring CI builds for the moment, the ordered number is computed
+from the values of the various parts of a version as they are constrained by the
+CSemVer spec. The math involved is not important for this discussion. Just that each
+Constrained Version is representable as a distinct integral value (63 bits actually).
+A CSemVer-CI build has two elements the base build and the additional 'BuildIndex' and
+'BuildName' components. This means the string, File version and ordered version
+numbers are confusingly different for a CI build. The ordered version number does NOT
+account for CI in any way. It is ONLY able to convert to/from a CSemVer. Thus, a
+CSemVer-CI has an ambiguous conversion. Should it convert the Patch+1 form in a string
+or the base build number?
 
 ### File Version Quad and UINT64
 A file Version quad is a data structure that is blittable as an unsigned 64 bit value.
@@ -153,10 +161,11 @@ Bits 1-63 are the same as the ordered version of the base build for a CI build a
 the same as the ordered version of a release build.
 
 ------
-<sup><a id="footnote_1">1</a></sup> Endianess of the platform does not matter as the bits are numbered as MSB->LSB
-and the actual byte layout is dependent on the target platform even though the bits
-are not. It is NOT safe to transfer a FileVersion (or Ordered version) as in integral
-value without considering the endianess of the source, target and transport mechanism,
-all of which are out of scope for this library and the CSemVer spec in general.
+<sup><a id="footnote_1">1</a></sup> Endianess of the platform does not matter for the
+purposes of discussion or this library, as the bits are numbered as MSB->LSB and the
+actual byte layout is dependent on the target platform even though the bits are not.
+It is NOT safe to transfer a FileVersion (or Ordered version) as in integral value
+without considering the endianess of the source, target and transport mechanism, all
+of which are out of scope for this library and the CSemVer spec in general.
 
 

src/Ubiquity.NET.Versioning.Build.Tasks.UT/AssemblyValidationTests.cs

@@ -15,6 +15,8 @@
 using Microsoft.Build.Utilities.ProjectCreation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using Ubiquity.NET.Versioning.Build.Tasks.UT.Support;
+
 namespace Ubiquity.NET.Versioning.Build.Tasks.UT
 {
     [TestClass]

src/Ubiquity.NET.Versioning.Build.Tasks.UT/BuildTaskErrorTests.cs

@@ -11,6 +11,8 @@
 using Microsoft.Build.Evaluation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using Ubiquity.NET.Versioning.Build.Tasks.UT.Support;
+
 namespace Ubiquity.NET.Versioning.Build.Tasks.UT
 {
     [TestClass]

src/Ubiquity.NET.Versioning.Build.Tasks.UT/BuildTaskTests.cs

@@ -7,10 +7,14 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
+using System.Text;
 
 using Microsoft.Build.Evaluation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using Ubiquity.NET.Versioning.Build.Tasks.UT.Support;
+
 namespace Ubiquity.NET.Versioning.Build.Tasks.UT
 {
     [TestClass]
@@ -52,8 +56,7 @@ public void GoldenPathTest( string targetFramework )
             // is not possible to know 'a priori' what the value will be..., additionally, the
             // CiBuildName is dependent on the environment. Other, tests validate the behavior of
             // those with an explicit setting...
-            string expectedFullBuildNumber = $"20.1.5-alpha.ci.{props.CiBuildIndex}.{props.CiBuildName}";
-            string expectedShortNumber = $"20.1.5-a.ci.{props.CiBuildIndex}.{props.CiBuildName}";
+            string expectedFullBuildNumber = $"20.1.5-alpha.0.0.ci.{props.CiBuildIndex}.{props.CiBuildName}";
             string expectedFileVersion = "5.44854.3875.59947"; // CI build
 
             Assert.IsNotNull(props.BuildMajor, "should have a value set for 'BuildMajor'");
@@ -123,6 +126,7 @@ public void BuildVersionXmlIsUsed( string targetFramework )
             // v20.1.5 => 5.44854.3880.52268 [see: https://csemver.org/playground/site/#/]
             // NOTE: CI build is Patch+1 for the string form
             // and for a FileVersion, it is baseBuild + CI bit.
+            // Non-prerelease double dash.
             string expectedFullBuildNumber = $"20.1.6--ci.ABCDEF12.ZZZ";
             string expectedFileVersion = "5.44854.3880.52269";
 
@@ -253,32 +257,54 @@ public void CiBuildInfoIsProcessedCorrectly( string targetFramework )
         }
 
         [TestMethod]
-        [DataRow("netstandard2.0")]
-        [DataRow("net48")]
-        [DataRow("net8.0")]
-        public void PreReleaseFixOfZeroNotShownIfNumber( string targetFramework )
+        [DynamicData(nameof(GetPrereleaseTestData))]
+        public void PreReleaseFixShownCorrectly( PrereleaseTestData data )
         {
+            // This test validates how pre-release number and fix are shown.
+            // For a CSemVer-CI these are always shown, even if 0. For a
+            // CSemVer, however, they are not shown if zero except the Number which
+            // is shown as 0 IFF Fix > 0.
+
             // NOT using BuildVersion.xml, all values set as globals to test handling of that
             var globalProperties = new Dictionary<string, string>
             {
                 [PropertyNames.BuildMajor] = "20",
                 [PropertyNames.BuildMinor] = "1",
                 [PropertyNames.BuildPatch] = "5",
                 [PropertyNames.PreReleaseName] = "delta",
-                [PropertyNames.PreReleaseNumber] = "1",
-                [PropertyNames.PreReleaseFix] = "0",
-                [PropertyNames.BuildTime] = "2025-06-02T10:15:48-07:00", // Format typical of commit date time stamp
-                [PropertyNames.CiBuildName] = "QRP", // Intentionally, not a standard value
+                [PropertyNames.PreReleaseNumber] = data.Number.ToString(CultureInfo.InvariantCulture),
+                [PropertyNames.PreReleaseFix] = data.Fix.ToString(CultureInfo.InvariantCulture),
+                [EnvVarNames.IsAutomatedBuild] = "true", // Not a local build (only relevant for a CI build)
             };
 
-            // compute build index from the time stamp to get the expected value of the index
-            // Technically, this is const as the time stamp itself is a const, but this saves on
-            // "magic numbers" and allows easier updates to validate a different time stamp.
-            var parsedBuildTime = DateTime.Parse(globalProperties[PropertyNames.BuildTime], CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind);
-            string expectedIndex = parsedBuildTime.ToBuildIndex();
+            string? expectedCiIndex = string.Empty;
+            string? expectedCiName = string.Empty;
+            string versionBase = "20.1.5-delta";
+
+            if(data.IsCI)
+            {
+                globalProperties[PropertyNames.BuildTime] = "2025-06-02T10:15:48-07:00"; // Format typical of commit date time stamp
+                globalProperties[PropertyNames.CiBuildName] = "QRP"; // Intentionally, not a standard value
+
+                // compute build index from the time stamp to get the expected value of the index
+                // Technically, this is const as the time stamp itself is a const, but this saves on
+                // "magic numbers" and allows easier updates to validate a different time stamp.
+                var parsedBuildTime = DateTime.Parse(globalProperties[PropertyNames.BuildTime], CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind);
+                expectedCiIndex = parsedBuildTime.ToBuildIndex();
+                expectedCiName = globalProperties[PropertyNames.CiBuildName];
+
+                // CI version strings are Patch+1!
+                versionBase = "20.1.6-delta";
+            }
+            else
+            {
+                // ensure generation is based on the test input and does NOT create
+                // a CI build version if it isn't supposed to.
+                globalProperties[EnvVarNames.IsReleaseBuild] = "true";
+            }
 
             using var collection = new ProjectCollection(globalProperties);
-            using var fullResults = Context.CreateTestProjectAndInvokeTestedPackage(targetFramework, collection);
+            using var fullResults = Context.CreateTestProjectAndInvokeTestedPackage(data.Tfm, collection);
             var (buildResults, props) = fullResults;
             Assert.IsTrue(buildResults.Success);
 
@@ -289,8 +315,25 @@ public void PreReleaseFixOfZeroNotShownIfNumber( string targetFramework )
             // is not possible to know 'a priori' what the value will be..., additionally, the
             // CiBuildName is dependent on the environment. Other, tests validate the behavior of
             // those with an explicit setting...
-            string expectedFullBuildNumber = $"20.1.6-delta.1.ci.{expectedIndex}.QRP";
-            string expectedFileVersion = "5.44854.3878.63541"; // CI Build (+1)
+            var expectedVersionbldr = new StringBuilder(versionBase);
+            string expectedPrerelString = data.Expected;
+            if(!string.IsNullOrWhiteSpace(expectedPrerelString))
+            {
+                expectedVersionbldr.Append('.')
+                                   .Append(expectedPrerelString);
+            }
+
+            if( data.IsCI )
+            {
+                expectedVersionbldr.Append(".ci")
+                                   .Append('.')
+                                   .Append(expectedCiIndex)
+                                   .Append('.')
+                                   .Append(expectedCiName);
+            }
+
+            string expectedFullBuildNumber = expectedVersionbldr.ToString();
+            FileVersionQuad expectedFileVersion = ExpectedFileVersion(data);
 
             Assert.IsNotNull(props.BuildMajor, "should have a value set for 'BuildMajor'");
             Assert.AreEqual(20u, props.BuildMajor.Value);
@@ -305,34 +348,63 @@ public void PreReleaseFixOfZeroNotShownIfNumber( string targetFramework )
             Assert.AreEqual("delta", props.PreReleaseName);
 
             Assert.IsNotNull(props.PreReleaseNumber, "Should have a value set for 'PreReleaseNumber'");
-            Assert.AreEqual((ushort)1u, props.PreReleaseNumber);
+            Assert.AreEqual(data.Number, props.PreReleaseNumber);
 
             Assert.IsNotNull(props.PreReleaseFix, "Should have a value set for 'PreReleaseFix'");
-            Assert.AreEqual((ushort)0u, props.PreReleaseFix);
+            Assert.AreEqual(data.Fix, props.PreReleaseFix);
 
             Assert.AreEqual(expectedFullBuildNumber, props.FullBuildNumber);
             Assert.AreEqual(expectedFullBuildNumber, props.PackageVersion);
 
-            Assert.AreEqual(globalProperties[PropertyNames.BuildTime], props.BuildTime);
-            Assert.AreEqual(expectedIndex, props.CiBuildIndex);
+            if( data.IsCI )
+            {
+                Assert.AreEqual(globalProperties[PropertyNames.BuildTime], props.BuildTime);
+            }
 
-            Assert.AreEqual("QRP", props.CiBuildName);
+            // Default for these is NULL (not specified) so these should match independent of CI
+            Assert.AreEqual(expectedCiIndex, props.CiBuildIndex);
+            Assert.AreEqual(expectedCiName, props.CiBuildName);
 
             Assert.IsNotNull(props.FileVersionMajor);
-            Assert.AreEqual(5, props.FileVersionMajor.Value);
+            Assert.AreEqual(expectedFileVersion.Major, props.FileVersionMajor.Value);
 
             Assert.IsNotNull(props.FileVersionMinor);
-            Assert.AreEqual(44854, props.FileVersionMinor.Value);
+            Assert.AreEqual(expectedFileVersion.Minor, props.FileVersionMinor.Value);
 
             Assert.IsNotNull(props.FileVersionBuild);
-            Assert.AreEqual(3878, props.FileVersionBuild.Value);
+            Assert.AreEqual(expectedFileVersion.Build, props.FileVersionBuild.Value);
 
             Assert.IsNotNull(props.FileVersionRevision);
-            Assert.AreEqual(63541, props.FileVersionRevision.Value);
+            Assert.AreEqual(expectedFileVersion.Revision, props.FileVersionRevision.Value);
 
-            Assert.AreEqual(expectedFileVersion, props.FileVersion);
-            Assert.AreEqual(expectedFileVersion, props.AssemblyVersion);
+            string expectedFileVersionString = expectedFileVersion.ToString();
+            Assert.AreEqual(expectedFileVersionString, props.FileVersion);
+            Assert.AreEqual(expectedFileVersionString, props.AssemblyVersion);
             Assert.AreEqual(expectedFullBuildNumber, props.InformationalVersion);
+
+            // Inline function to support simpler conversion of parameters to
+            // expected FileVersionQuad
+            //
+            // v20.1.5-delta => 5.44854.3878.63340 [see: https://csemver.org/playground/site/#/]
+            // NOTE: CI build is Patch+1
+            static FileVersionQuad ExpectedFileVersion( PrereleaseTestData d )
+            {
+                FileVersionQuad retVal = d.PrereleaseIndex switch
+                {
+                    0 => new(5, 44854, 3878, 63340), // v20.1.5-delta[0.0]
+                    1 => new(5, 44854, 3878, 63342), // v20.1.5-delta.0.1
+                    2 => new(5, 44854, 3878, 63540), // v20.1.5-delta.1[.0]
+                    3 => new(5, 44854, 3878, 63542), // v20.1.5-delta.1.1
+                    _ => throw new InvalidOperationException("Unknown pre-release index")
+                };
+
+                if(d.IsCI)
+                {
+                     retVal = retVal with { Revision = (ushort)(retVal.Revision + 1u) };
+                }
+
+                return retVal;
+            }
         }
 
         [TestMethod]
@@ -371,6 +443,13 @@ public void ValidateVersionFormatting( bool isPreRelease, bool isCiBuild, bool i
                 globalProperties[PropertyNames.CiBuildIndex] = "MyIndex"; // Intentionally not a standard value
                 globalProperties[PropertyNames.CiBuildName] = "QRP"; // Intentionally, not a standard value
                 string ciSuffix = isPreRelease ? ".ci.MyIndex.QRP" : "--ci.MyIndex.QRP";
+
+                if (isPreRelease)
+                {
+                    // CI Builds Always include both parts
+                    expectedFullBuildNumber += $".{globalProperties[PropertyNames.PreReleaseFix]}";
+                }
+
                 expectedFullBuildNumber += ciSuffix;
             }
             else
@@ -454,11 +533,62 @@ public void ValidateVersionFormatting( bool isPreRelease, bool isCiBuild, bool i
             // NOTE: CI build is +1 (FileVersionRevision)!
             static FileVersionQuad ExpectedFileVersion( bool isPreRelease, bool isCiBuild )
             {
-                FileVersionQuad retVal = isPreRelease ? new(5, 44854, 3876, 34610) : new(5, 44854, 3878, 23338);
+                FileVersionQuad retVal = isPreRelease
+                                       ? new(5, 44854, 3876, 34610)
+                                       : new(5, 44854, 3878, 23338);
 
                 // NOTE: ODD numbered revisions are for CI builds.
-                return isCiBuild ? retVal with { Revision = (ushort)(retVal.Revision + 1u) } : retVal;
+                return isCiBuild
+                     ? retVal with { Revision = (ushort)(retVal.Revision + 1u) }
+                     : retVal;
             }
         }
+
+        private static IEnumerable<PrereleaseTestData> GetPrereleaseTestData()
+        {
+            return from tfm in TargetFrameworks
+                   from num in NumberOrFixArray
+                   from fix in NumberOrFixArray
+                   from isCI in BooleanValue
+                   select new PrereleaseTestData(tfm, num, fix, isCI);
+        }
+
+        private static IEnumerable<string> TargetFrameworks => ["netstandard2.0", "net48", "net8.0"];
+
+        private static readonly bool[] BooleanValue = [true, false];
+
+        private static readonly byte[] NumberOrFixArray = [0, 1];
+    }
+
+    public readonly record struct PrereleaseTestData(string Tfm, byte Number, byte Fix, bool IsCI)
+    {
+        internal string Expected
+        {
+            get
+            {
+                // CSemVer-CI ***ALWAYS*** includes the build numbers for pre-release versions
+                // this ensures correct sort ordering of CI builds (which are POST-RELEASE)
+                if(IsCI)
+                {
+                    return $"{Number}.{Fix}";
+                }
+
+                // Non CI Might include the release number of zero (IFF Fix > 0)
+                var bldr = new StringBuilder();
+                if(Number > 0 || Fix > 0)
+                {
+                    bldr.Append(Number);
+                    if(Fix > 0)
+                    {
+                        bldr.Append('.')
+                            .Append(Fix);
+                    }
+                }
+
+                return bldr.ToString();
+            }
+        }
+
+        internal int PrereleaseIndex => (Number << 1) + Fix;
     }
 }

src/Ubiquity.NET.Versioning.Build.Tasks.UT/CreateVersionInfoTaskErrorTests.cs

@@ -12,6 +12,8 @@
 using Microsoft.Build.Evaluation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using Ubiquity.NET.Versioning.Build.Tasks.UT.Support;
+
 namespace Ubiquity.NET.Versioning.Build.Tasks.UT
 {
     [TestClass]

src/Ubiquity.NET.Versioning.Build.Tasks.UT/ParseBuildVersionXmlTaskErrorTests.cs

@@ -13,6 +13,8 @@
 using Microsoft.Build.Evaluation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using Ubiquity.NET.Versioning.Build.Tasks.UT.Support;
+
 namespace Ubiquity.NET.Versioning.Build.Tasks.UT
 {
     [TestClass]