Replace Nunit with xUnit

[asos-gurpreetsingh]

No description provided.

[SeanFeldman]

Thank you, @asos-gurpreetsingh. The unit tests in this project are... insufficient to say the least.
Looking at the changes to NUnit, I'm inclined to decline this. Here's why

This new NUnit API is attempting to mimic Fluent Assertions but looks off. Take the following example:

Assert.That(false, Is.EqualTo(nonExistingValueAsFalse));

Every assertion has to have Assert.That() as a container method. With Fluent Assertion, the same would be more readable.

nonExistingValueAsFalse.Should().BeFalse();

The benefits IMO are

1. More fluid
2. No "container method"
3. Reads better for humans

I can support Fluent Assertions if you'd like. However, replacing an existing clunky NUnit API with another variant is not worth it.

[asos-gurpreetsingh]

Thanks @SeanFeldman , are we saying that if we update the Nunit package then we want to use fluent assertions otherwise we will never update Nunit package ? I am happy to use fluent assertions, I only took the path of least change in order to update nunit

[SeanFeldman]

Is the new API mandatory for the latest version of NUnit?

[asos-gurpreetsingh]

The old methods were not available when I updated. Anyway I will refactor to use fluent assertions, I would prefer that too

[SeanFeldman]

Thanks, @asos-gurpreetsingh.
If you're already at it, we might as well swap NUnit with XUnit. It's a more commonly used library this days, including MSFT projects.

[asos-gurpreetsingh]

Sure @SeanFeldman I will get the sorted.

[asos-gurpreetsingh]

@SeanFeldman I have migrated this to XUnit. Could you please also have a look at the workflow (not my strong suite)

[ErikMogensen]

I like the switch to xUnit and the new assertions.

[SeanFeldman]

A couple of minor things. The xUnit changes look good.

[SeanFeldman]

I don't recall wherever PR build should have a version or not. @ErikMogensen do you remember?

[asos-gurpreetsingh]

I don't recall wherever PR build should have a version or not. @ErikMogensen do you remember?

I added default input for workflow manual build that is what shows version number here, without this i cannot test build in feature branch, it could not find version release-version which is set as env variable.

[ErikMogensen]

I don't recall wherever PR build should have a version or not. @ErikMogensen do you remember?

They do not have a version number so this is correct.

[ErikMogensen]

I tested to build this branch using dotnet build -c RELEASE -p:GenerateAssemblyInfo=true,AssemblyVersion=0.1.2.3,InformationalVersion=0.0.1.1. It sets File version to the same value as the Assembly version, which we don't want. Assembly version should be set per assembly. File version should be the same on all assemblies created by the build.

[SeanFeldman]

@asos-gurpreetsingh, do you plan to complete this one? Erik pointed out one last thing that needs attention.

[asos-gurpreetsingh]

@asos-gurpreetsingh, do you plan to complete this one? Erik pointed out one last thing that needs attention.

Hi Yes I will pick this weekend, i was out on holiday

[asos-gurpreetsingh]

I tested to build this branch using dotnet build -c RELEASE -p:GenerateAssemblyInfo=true,AssemblyVersion=0.1.2.3,InformationalVersion=0.0.1.1. It sets File version to the same value as the Assembly version, which we don't want. Assembly version should be set per assembly. File version should be the same on all assemblies created by the build.

In your test you have used very different version numbers, if our objective is to use something like 1.2.3.0 as assembly version and 1.2.3.566 as file version then this command would do that. But if we really want to set different versions like in you example then we can use another Property "FileVersion" for that

[ErikMogensen]

Yes, we want to be able to set different versions. Thinking of this, I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline.

[ErikMogensen]

There is also an issue with the build.

[asos-gurpreetsingh]

Yes, we want to be able to set different versions. Thinking of this, I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline.

Can I ask why ?
We could use git-version to drive version numbers.

[asos-gurpreetsingh]

There is also an issue with the build.

build is fixed.

[ErikMogensen]

Yes, we want to be able to set different versions. Thinking of this, I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline.

Can I ask why ? We could use git-version to drive version numbers.

Of course. Assembly version is used by the .NET runtime to determine and verify it is loading the intended assembly. The assembly number of a dependent assembly is stored in the manifest of the calling assembly. By default it will only load that assembly.

File version may be used for installers to determine which version an assembly has so the numbers have different purposes.

By setting all the assemblies built in a build to have the same file version it is easy to tell for an installer and a human which build a certain assembly belongs to. Typically you do not want to change the assembly version in every build since that would make assemblies between two different builds incompatible. The exception to this if you have made changes in the assembly that lost its backward combability, then it should increase its assembly number.

Does this PR still set the assembly version to the same value on all assemblies?

[asos-gurpreetsingh]

Yes, we want to be able to set different versions. Thinking of this, I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline.

Can I ask why ? We could use git-version to drive version numbers.

Of course. Assembly version is used by the .NET runtime to determine and verify it is loading the intended assembly. The assembly number of a dependent assembly is stored in the manifest of the calling assembly. By default it will only load that assembly.

File version may be used for installers to determine which version an assembly has so the numbers have different purposes.

By setting all the assemblies built in a build to have the same file version it is easy to tell for an installer and a human which build a certain assembly belongs to. Typically you do not want to change the assembly version in every build since that would make assemblies between two different builds incompatible. The exception to this if you have made changes in the assembly that lost its backward combability, then it should increase its assembly number.

Does this PR still set the assembly version to the same value on all assemblies?

Thanks, @ErikMogensen . I understand now why one could want different versions but I am not sure we SBE uses it with that intent. But with current state of build task "dotnet build ....." allows us to use different versions for file and assembly but that will be applied to all assemblies. I don't think this we ever set up to use different file versions for each project. That can be done by setting those in the project files. Secondly if that is what we want then we can pick this up in another change and not this one.

[ErikMogensen]

Thanks, @ErikMogensen . I understand now why one could want different versions but I am not sure we SBE uses it with that intent.

We should probably have updated the assembly numbers in some PRs.

But with current state of build task "dotnet build ....." allows us to use different versions for file and assembly but that will be applied to all assemblies. I don't think this we ever set up to use different file versions for each project. That can be done by setting those in the project files.

We don't want to have different file versions for assemblies in the same build so that's fine.

Secondly if that is what we want then we can pick this up in another change and not this one.

All work on this tool is voluntarily so there is quite a big risk that such a change will never be created.

What is the problem with using msbuild for building?

[asos-gurpreetsingh]

Thanks, @ErikMogensen . I understand now why one could want different versions but I am not sure we SBE uses it with that intent.

We should probably have updated the assembly numbers in some PRs.

But with current state of build task "dotnet build ....." allows us to use different versions for file and assembly but that will be applied to all assemblies. I don't think this we ever set up to use different file versions for each project. That can be done by setting those in the project files.

We don't want to have different file versions for assemblies in the same build so that's fine.

Secondly if that is what we want then we can pick this up in another change and not this one.

All work on this tool is voluntarily so there is quite a big risk that such a change will never be created.

What is the problem with using msbuild for building?

Apologies @ErikMogensen if I am missing something. when you say "I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline. " what is that to do with MSBuild or dotnet build ? and secondly "using a file belonging to the project of the affected assembly" sounds contradicting to "We don't want to have different file versions for assemblies". Could you please elaborate. Thanks

[ErikMogensen]

Apologies @ErikMogensen if I am missing something. when you say "I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline. " what is that to do with MSBuild or dotnet build ?

I assumed dotnet build cannot handle assembly version set in a project since you changed the way assembly versions were set. But if it is possible to set assembly versions using files belonging to each project and still use dotnet build, it would be great.

secondly "using a file belonging to the project of the affected assembly" sounds contradicting to "We don't want to have different file versions for assemblies". Could you please elaborate. Thanks

It is about File version vs Assembly version. We want to have the same File version on all assemblies created by the build. We want Assembly version set separately for each assembly.

Just ask again if it is unclear.

[asos-gurpreetsingh]

Apologies @ErikMogensen if I am missing something. when you say "I want the assembly version to be set using a file belonging to the project of the affected assembly instead of being set in the pipeline. " what is that to do with MSBuild or dotnet build ?

I assumed dotnet build cannot handle assembly version set in a project since you changed the way assembly versions were set. But if it is possible to set assembly versions using files belonging to each project and still use dotnet build, it would be great.

secondly "using a file belonging to the project of the affected assembly" sounds contradicting to "We don't want to have different file versions for assemblies". Could you please elaborate. Thanks

It is about File version vs Assembly version. We want to have the same File version on all assemblies created by the build. We want Assembly version set separately for each assembly.

Just ask again if it is unclear.

Thanks @ErikMogensen . In that case file version can be set by dotnet build, but setting different assembly versions for each assembly would mean updating project files manually when needed. What assembly version should we set to start with ? when would it be changed (on what basis)

[ErikMogensen]

Thanks @ErikMogensen . In that case file version can be set by dotnet build, but setting different assembly versions for each assembly would mean updating project files manually when needed.

Great!

What assembly version should we set to start with ?

Look with the ones that are in the main branch. (I believe all are 1.0.0,0 which is most likely not right.) Since this PR changed the .NET version, increment the major version digit (the first one).

when would it be changed (on what basis)

They should be increased when there is new release of the assembly with an interface that is not backwards compatible. By interface I mean, public methods, properties, interfaces, events and delegates. (I may have forgotten one or two.) Also, if the runtime requirements change I consider that a change that should increase the version number.

[asos-gurpreetsingh]

@ErikMogensen I have set set the assembly versions to 1.0.0.1 in all projects and build script will provide the file version. We should be able to change the assembly versions from projects now. I have updated the tests to reflect that too. Please check if this works as you expected. Could you also confirm that the publish process is not affected by this. (i have checked and it does not look like it should be affected, but please check).

[ErikMogensen]

Well done @asos-gurpreetsingh !