(GH-2456) Correctly assert version number for tagged commit

[gep13]

Description

When running on a CI system, like GitHub Actions, when building a tagged commit, rather than asserting something like 0.2.0, which is expected, GitVersion asserts a version number like 0.3.0-tags-0-2-0-0001. An example of this can be seen here:

https://github.com/cake-contrib/Cake.StrongNameSigner/runs/1779759425?check_suite_focus=true#step:5:217

A fix has been made to the code to only increment the semantic version number when on a tagged commit, if the current sha of the repository is different to the sha of the tagged semantic version.

This was discussed on a Twitch stream with @arturcic

Related Issue

#2456

Motivation and Context

How Has This Been Tested?

I have tested this code locally on my machine, and added unit tests to cover the code that has been changed.

Screenshots (if appropriate):

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[arturcic]

@gep13 I wonder if we need to add a similar test for AzurePipelines

[gep13]

I actually had the same thought. I can break the test up in a similar way to the PullRequestBuildAgentTewt class.

Just away to make the kids supper just now, but should be able to get this updated later tonight.

Does the single test, and code change make sense?

[gep13]

In general I mean? Is this a sensible change?

[arturcic]

In general I mean? Is this a sensible change?

I think it makes sense. Would be great to add an additional test in the Core.Test project as the change is made in the Core

This one is more like an integration test.

[arturcic]

Would be great to add an additional test in the Core.Test

in TagCheckoutScenarios

[gep13]

I started looking there, but the problem only occurs when the environment variables come into play, which only seemed to be in the integration tests.

If the environment variables aren't passed in, the correct version number is asserted without the code change.

[arturcic]

I started looking there, but the problem only occurs when the environment variables come into play, which only seemed to be in the integration tests.

If the environment variables aren't passed in, the correct version number is asserted without the code change.

But is the environment variable is the issue or it's value? I tend to think it's the value, so in that case we might be able to reproduce the value in a test

[gep13]

I am not sure that I follow 😢

This test:

GitVersion/src/GitVersion.Core.Tests/IntegrationTests/TagCheckoutScenarios.cs

Lines 34 to 49 in 6c8e199

	public void GivenARepositoryWithTwoTagsAndADevelopBranch()
	{
	using var fixture = new EmptyRepositoryFixture();
	const string firstVersion = "1.0";
	const string hotfixVersion = "1.0.1";
	fixture.MakeACommit($"init {TestBase.MainBranch}");
	fixture.ApplyTag(firstVersion);
	fixture.MakeACommit("hotfix");
	fixture.ApplyTag(hotfixVersion);
	fixture.BranchTo("develop");
	fixture.MakeACommit("new feature");
	fixture.Checkout(hotfixVersion);
	fixture.BranchTo("tags/1.0.1");
	fixture.AssertFullSemver(hotfixVersion);

essentially tests the same thing as I am testing with my new test. This test works when running GitVersion locally, without any CI's involved.

As soon as you add a CI into the mix, the wrong version number is asserted.

I believe this is due to the addition work that it done to normalize the repository prior to asserting the version number.

[arturcic]

I am not sure that I follow 😢

This test:

GitVersion/src/GitVersion.Core.Tests/IntegrationTests/TagCheckoutScenarios.cs

Lines 34 to 49 in 6c8e199

	public void GivenARepositoryWithTwoTagsAndADevelopBranch()
	{
	using var fixture = new EmptyRepositoryFixture();
	const string firstVersion = "1.0";
	const string hotfixVersion = "1.0.1";
	fixture.MakeACommit($"init {TestBase.MainBranch}");
	fixture.ApplyTag(firstVersion);
	fixture.MakeACommit("hotfix");
	fixture.ApplyTag(hotfixVersion);
	fixture.BranchTo("develop");
	fixture.MakeACommit("new feature");
	fixture.Checkout(hotfixVersion);
	fixture.BranchTo("tags/1.0.1");
	fixture.AssertFullSemver(hotfixVersion);

essentially tests the same thing as I am testing with my new test. This test works when running GitVersion locally, without any CI's involved.
As soon as you add a CI into the mix, the wrong version number is asserted.

I believe this is due to the addition work that it done to normalize the repository prior to asserting the version number.

Ok, you're right. Will you add the AzurePipelines test as well?

[gep13]

@arturcic said...
Will you add the AzurePipelines test as well?

Yip, I can do that, but it won't be until later this evening at the earliest.

[gep13]

@arturcic an additional test has been added to cover Azure Pipelines.

[arturcic]

@gep13 thank you so much for your contributions 👍

[github-actions]

🎉 This issue has been resolved in version 5.6.5 🎉
The release is available on:

• GitHub Release
• Chocolatey Package
• .Net Global Tool

Your GitReleaseManager bot 📦🚀