Add failing test for bug in GetLatestMaskedVersion #77
Conversation
| // Now the real test; it should still return the same result even if the versions are not in the correct order | ||
| var reversedVersions = versions.ToList(); | ||
| reversedVersions.Reverse(); | ||
| Assert.AreEqual("0.0.7+branchA.1", mask.GetLatestMaskedVersion(reversedVersions)!.ToString()); |
There was a problem hiding this comment.
This currently returns 0.0.7+branchA instead of A.1
There was a problem hiding this comment.
The Semver spec says that version metadata (everything after +) shouldn't affect precedence:
Build metadata MUST be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence. Examples: 1.0.0-alpha+001, 1.0.0+20130313144700, 1.0.0-beta+exp.sha.5114f85, 1.0.0+21AF26D3----117B344092BD.
There was a problem hiding this comment.
Be that as it may, the side-effect of indeterminate precedence is that in Octopus, when it tries to do auto-deploy or other things the release it selects is nondeterministic where two builds with the same version but different metadata happens. It just picks whatever happens to come out of SQL server first which is not reliable from a customer perspective.
If we don't want deterministic ordering in our library here, then we'll need to make some compensating change for it in Octopus instead.
There was a problem hiding this comment.
The problem here stems from OctopusVersionMask.CompareTo(object). We have tests here ensuring that version metadata doesn't affect versioning (OctopusVersionCompareTests.TestVersionComparisons(...)). I'm inclined to leave the tests as-is, and instead create a new GetLatestMaskedVersions method that returns all versions considered to be "latest". Then we can let Octopus decide how to handle that. What are your thoughts?
No description provided.