stride icon indicating copy to clipboard operation
stride copied to clipboard

[WIP] Update and fix test projects

Open Ethereal77 opened this issue 2 years ago • 7 comments

PR Details

This PR updates the test project dependencies to their latest versions, and fixes some issues that were difficulting the compilation of sample projects (with the intent of also testing them).

Description

  • Updates Xunit and related dependencies (like Xunit runners, etc).
  • Updates Visual Studio Test SDK.
  • Updates TeamCity Test Adapter.
  • Changes the Stride version in the samples / templates to match the version specified in SharedAssemblyInfo.cs. Rationale: When developing and testing, I think it's more desirable that the samples are always resolving the same version of Stride you are building, so they can also serve to check potential API conflicts and changes, and regressions in functionality or performance. I even sometimes set a higher version myself for testing (like v4.2.0.0 for example).
  • Fixes some issues in the samples / templates related to API breaking changes, or missing files, or incorrect paths.
  • Removes the RuntimeIdentifier from some of the test projects, as it was making the tests to fail. Maybe the problem is not with the specific RID but with assembly resolution or native reference resolution?

Motivation and Context

Test projects failing are a common theme when many people try to build Stride from source the first time, and always the answer is to disregard the errors as they are from test projects. But test projects being in a good state will be better as errors will truly reflect something is wrong and not simply the tests are wrong. Also, having the test suite in a good state will allow us to better detect regressions and failures when, for example, rewriting effects or renderers, replacing SharpDX, etc.

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [ ] My change requires a change to the documentation.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.

Additional comments

  1. For the project xunit.runner.stride I had to remove the SDK reference to MSBuild.Sdk.Extras. This package introduced some extra targets and utilities for multi-targeting and multi-RID projects, but many of those things are already integrated now in .NET 6/7/8 SDK, and maybe some others we don't need them anymore. Do we still need it? Can it be replaced entirely with just Microsoft.NET.Sdk?

  2. This PR is still a Work in Progress because I think it has to be tested first by more people than me. But also because the project Stride.Core.ProjectTemplating has a weird issue that haven't allowed me to make the tests in Stride.Core.ProjectTemplating.Tests and Stride.Samples.Tests pass. The Project Templating project uses a custom version of Mono.TextTemplating for expanding the project templates (in T4 format) and its files (the ones that are templates) to a destination directory. That version of Mono.TextTemplating is outdated and compiled against net45 and netstandard. However there should not be any problem with this. But when running the tests, it throws this ArgumentException to me:

    'GeneratedTextTransformation143a7b9e.dll' was already in the collection. Arg_ParamName_Name
    

    I've tried to modernize it with the most recent version of Mono.TextTemplating that supports .NET 5+, and I've had success to certain point, but have also reached a point of pain in that the templates compile and begin running but says Properties.Tutu does not exist (Properties is a dynamic property that gives access to an ExpandoObject), even if it should work. I've tested this in separate projects and it should work fine. A possible problem seems to be that accessing that Tutu field throws a RuntimeBinderException that somehow cancels the whole text templating attempt. However, I've checked the sources of both Stride and those decompiled from the custom Stride.Mono.TextTemplating package, and they do it the same way. So if it did not fail in the past but fails now, something is making it fail now. Just I haven't been able to find what. Recently Microsoft has announced a T4 tool entirely written and compatible with modern .NET runtimes. The Text Templating engine it uses however is (for now) not open source, and also not available as a NuGet package, so can't use it reliably to check if it helps here.

Ethereal77 avatar Jun 25 '23 17:06 Ethereal77

Also, if the problem with the ExpandoObject can be fixed and the tests pass succesfully, I can contribute also my modernized version of Stride.Core.TextTemplating, and discard the dependency to the custom Stride.Mono.TextTemplating.

Ethereal77 avatar Jun 25 '23 17:06 Ethereal77

Adding @xen2 into the mix to see if he has some insight for the additional comments you wrote in the OP

Eideren avatar Sep 24 '23 12:09 Eideren

Changes the Stride version in the samples / templates to match the version specified in SharedAssemblyInfo.cs. Rationale: When developing and testing, I think it's more desirable that the samples are always resolving the same version of Stride you are building, so they can also serve to check potential API conflicts and changes, and regressions in functionality or performance. I even sometimes set a higher version myself for testing (like v4.2.0.0 for example).

Some reasons it was not the case:

  • No need to deploy new samples for every single minor release (esp. since they are large with assets).
  • If samples are not actually upgraded, even if they are tagged with a specific version, they might just be upgraded to an older version, which can be misleading.
  • Asset upgrade will be much less tested.
  • No need to force dev to upgrade/commit samples on every release.

However, it's fine to upgrade them once in a while, but the idea was not to make it mandatory on every release/bump, as long as proper asset upgrader exist (which should be there for user project anyway). So I think the previous separate versioning was a better match.

Another point: samples used to be compiled, and even tested/run on teamcity, but it's not the case anymore. It should be setup properly again at some point (not in this PR of course).

xen2 avatar Oct 16 '23 12:10 xen2

No need to deploy new samples for every single minor release (esp. since they are large with assets).

I'm not talking about deploying the samples with every change. Just for the samples in the solution to be up-to-date with the Stride assemblies themselves, to ease testing and development. The samples can be manually published at its own cadence, for example, when they are expanded or significantly modified.

Asset upgrade will be much less tested.

This is true.

No need to force dev to upgrade/commit samples on every release.

Same as above. Samples would update at a mantainers-determined cadence, or when deemed necessary.

Another point: samples used to be compiled, and even tested/run on teamcity, but it's not the case anymore. It should be setup properly again at some point (not in this PR of course).

This is the main reason for this whole PR. So test projects can test the latest samples with the latest API to catch regressions and bugs, and so they can be again run automatically in TeamCity.

Ethereal77 avatar Oct 17 '23 10:10 Ethereal77

I'm not talking about deploying the samples with every change. Just for the samples in the solution to be up-to-date with the Stride assemblies themselves, to ease testing and development. The samples can be manually published at its own cadence, for example, when they are expanded or significantly modified.

Forgive me for the misunderstanding, I thought you changed versioning of Stride.Samples.Templates to not have its own version but use GameStudio version (forcing us to updating every release). I looked again at the PR and noticed this is not the case. Doing a sample upgrade (just like you did) once in a while is totally fine!

However, could you please bump those two files to 4.1.0.5/4.1.0.5-dev respectively? https://github.com/stride3d/stride/blob/master/sources/editor/Stride.Samples.Templates/ThisPackageVersion.PackageBuild.cs https://github.com/stride3d/stride/blob/master/sources/editor/Stride.Samples.Templates/ThisPackageVersion.DevBuild.cs

xen2 avatar Oct 17 '23 10:10 xen2

Just to make sure, I don't see any asset changed, is that because there was no specific asset upgrader since last sample update, or you didn't open the StrideSamples.sln with GameStudio? (wondering if you did update by manually editing file or using editor)

xen2 avatar Oct 17 '23 10:10 xen2

Just to make sure, I don't see any asset changed, is that because there was no specific asset upgrader since last sample update, or you didn't open the StrideSamples.sln with GameStudio? (wondering if you did update by manually editing file or using editor)

I simply changed the version in those files. I haven't opened the StrideSamples.sln with GameStudio. Should I?

could you please bump those two files to 4.1.0.5/4.1.0.5-dev respectively?

Of course! Is this the current Stride version in SharedAssemblyInfo.cs once I rebase?

Ethereal77 avatar Oct 18 '23 08:10 Ethereal77