Make contribution easier in the Reqnroll repo

[gasparnagy]

I guess anyone who tried to contribute to the main repo faced different issues that made the work harder.

I would like to take it as a top priority for myself to improve this. I have seen several problems myself as well, but I am interested to your comments as well:

What was hindering you when working with the Reqnroll solution?

Note: The compilation / locking issue seems to be fixed now by #64 (see #38)

CC @reqnroll/core-team

[ajeckmans]

Definitely not being able to run the test suite. Even after the changes made in #64 I cannot get all of the tests to succeed, which makes making changes to the core a lot more difficult.

There is also a dependency on having Visual Studio installed (through vswhere) which I think we might be able to eliminate as well.

[SabotageAndi]

Background info about vswhere usage:
This is necessary to find the msbuild executable for building the projects. We build it with msbuild, which is a .NET Framework binary, to test the compilation as in Visual Studio. Building within Visual Studio is still done in .NET Framework. This is also true for .NET (Core) projects.

On Linux I think we don't run the msbuild tests. Or we use the mono version. I can't remember anymore.

[gasparnagy]

OK. So maybe we should just:

• make clear what tests need this (so that they can be excluded)
• maybe provide a backup mechanism to override the MsBuild path with an environment variable an if that is present we don't use vswhere

@ajeckmans do you have MsBuild in a clean JetBrains dev machine? Where is it?

[ajeckmans]

JetBrains Rider comes packaged with a msbuild version. See the bottom two entries. However people who develop using VC Code might only have the dotnet/sdk entry. Whereas people with Visual Studio installed will probably have the dotnet\sdk entry as well as the Visual Studio entries.

[Tiberriver256]

I started up a bug here if we want to continue the conversation in a thread dedicated to VSWhere.

[Tiberriver256]

I started #76 to track getting tests to run locally. It seems like a big lift, much too big to do in a single PR, so I made a mega issue to track progress.

[gasparnagy]

One of my issues is/was the error reporting on CI. Initially we used dorny/test-reporter@v1, but that does not work with PRs. Then on the main repo we replaced it with EnricoMi/publish-unit-test-result-action@v2, but that only works on linux builds, so it cannot be used for Reqnroll.VisualStudio.

Now my next attempt is to try https://github.com/Tyrrrz/GitHubActionsTestLogger: I have introduced that to the Reqnroll.VisualStudio repo and pretty happy with it:

• It does not use GitHub action checks, so no security problem with PR
• It includes a nice test report on the "summary" page of the action; see sample with a simulated error:
  • failed tests with details in the annotations: https://github.com/reqnroll/Reqnroll.VisualStudio/actions/runs/8187761847
  • detailed report in "build summary": https://github.com/reqnroll/Reqnroll.VisualStudio/actions/runs/8187761847#summary-22388947505
• It annotates the PR source with the error source (works even with feature files!); see sample with a simulated error: https://github.com/reqnroll/Reqnroll.VisualStudio/pull/10/files

If you are happy with this, I would try to apply it to the main repo as well. CC @reqnroll/core-team

[ajeckmans]

Oh that is nice indeed!

[gasparnagy]

@ajeckmans do you have time maybe to apply it to all other tests in the Reqnroll repo? I did that for the SystemTests already (and also for the tests of Reqnroll.VisualStudio), based on that it should be ok.

[gasparnagy]

I enabled GitHubActionsTestLogger as part of #105

[gasparnagy]

We all see that testing is a crucial part of improving the contribution experience. As fas a I see one of the problems that currently almost all of the tests work in a combinatorial fashion: they target multiple frameworks and the Specs project even targets multiple test runners.

The reasons are clear: we would like to make sure that Reqnroll works on all supported platforms with all test runners, and this is a valid expectation. But with the current approach this generates too many overlaps, too many tests and slow test execution. In fact it is even controversial, because due to the complexity and the slow execution of test, we don't actually run them in multiple targets - the CI currently runs them only with .NET 6.0 and probably no one does it locally either regularly.

My imagined test strategy is the following:

1. We create a "system test" suite (standard MsTest), that focuses on

• smoke testing (works end-to-end)
• portability testing (works on all supported .NET platform)
• test execution framework generator end-to-end testing (the code we generate for xUnit/MsTest/NUnit works)

2. All other tests should focus on functional testing and should run on a single .NET platform (always the latest .NET platform targeted by Reqnroll core, currently this is .NET 6.0, but soon it will be .NET 8.0)
3. Particularly our "specs" test suite should be refactored to become BDD specification - they should concentrate on the capabilities of Reqnroll and not to test them in all combinations. For that this project has to be changed to run the scenarios of the tests with an in-proc runner, not end-to-end.
4. We need to review all other test projects and simplify them if possible. Having "Reqnroll projects" as tests, like for the external plugin is not practical, because of the circularity of dependencies (let's handle that only for Specs). Those should be converted to unit tests or made as external examples in a separate repo.
5. I prefer not to have separate test projects for small components, therefore the unit tests of the various plugins should also be merged (currently external data and verify plugin has separate test projects). I would merge them to the "PluginTests" project.

I have started to work on the "system tests" concept (it's in this PR) and will work on the rework of the "specs" project as these are quite connected. (This is why I asked @Tiberriver256 to skip Specs in #76.)

Any comments are welcome.

[clrudolphi]

there are TODOs to add more tests

Would you like assistance in completing these TODOs? Happy to help. Point out what I/we can work on without stepping on each other.

[gasparnagy]

I'm on vacation until the 8th, so will not do coding until that date (I can review PRs and answer the questions).
So feel free to take some of them.

The simplest is probably the one with the different outcomes: there my idea was to add a helper that creates a step def for each, eg "When the step passes" or "When the step fails" and then you can make a sample feature file that has one scenario of each (you can use it as resource as well, like I do with the GenerationAllIn files).

The ones with hooks might be also easier, because we have such tests in "specs" and there is an infrastructure there for asserting if a hook was executed. But I haven't checked that in detail yet.

The others might be trickier, I did not have much plan for those yet. But maybe by that time you will spot out a useful solution.

[clrudolphi]

I'll take a look.
Are these going to replace tests that already exist elsewhere (such as in Specs)? If so, would it be appropriate to look there to confirm scope of test conditions and/or to find inspiration on how to structure the scenarios and steps code?

[gasparnagy]

@clrudolphi Yes and no. The "Specs" contains a bunch of functional tests and automates them as end-to-end (system) tests. What we do now is to split this. In long term, Specs will be only functional tests automated using an in-memory executor (no invokes to dotnet, msbuild; no code generation, no external test execution frameworks), so the scenarios there will more-or-less remain the same, but they will be much faster.

To be able to do that, we have now the system tests, that verifies everything that cannot be tested with an in-memory execution (.net framework compatibility, test execution frameworks). But the tests here are not functional: they are not testing if the function works correctly, just make sure that it generally works, and we would like to keep the scope of system tests as small as possible. So we should only test something in the system tests if it cannot be tested elsewhere (in unit tests or in functional "specs" scenarios).

For example, testing whether before/after test run and before/after feature/scenario is invoked has to be verified in system tests, because they are triggered by test execution framework infrastructure that is used by the generated code. So these will go to system tests, but we don't need to test if the parameter injection works for those hooks, because that is Reqnroll functionality that can be tested in unit/specs tests. Also we don't need to test the before/after step and step block hook in system tests, because that is triggered by Reqnroll internally and can be tested elsewhere.

The "specs" project can be used as inspiration regarding to how something can be automated (and make assertions). It also uses the "TestProjectGenerator" which is a very feature-rich infrastructure for such end-to-end testing and it has a lot of cool stuff that can be used. But you have to be more careful with taking inspiration from the tests themselves, because they concentrate on the functionality and you would need to pick those elements from it that has to be moved to the system tests. So it's a splitting work at the end.

I hope this made it clearer.

[mbhoek]

Re: 4 and 5 I'll try to migrate my original tests to Reqnroll in a way that you described. Maybe we can use that as a template for the other plugins. I'll ping you once they're in the PR (#94).

[gasparnagy]

Another issues for contribution were the problem of cached packages. This issue we have discovered with @livioc on #54: If you modify the generation part (e.g. MsTestV2GeneratorProvider) and rerun the related "specs" test, the test will pick up the previously generated nuget package from the package cache and therefore the test will not reflect your changes (you still see the old behavior).

The reason for this is that these E2E test use the generated NuGet packages for generating the sample projects. Between two compilations these packages have the same version number, therefore if a version has been used, nuget automatically caches that in the local nuget cache (C:\Users<your-user>.nuget\packages), so the subsequent execution will re-use the cached version. The solution was to manually clean the Reqnroll related packages from the local nuget cache.

There has been tries to avoid this problem: in SpecFlow we tried to generate commit specific nuget versions (this caused other problems and did not solve the changes within commits) and also there was a code that tried to delete the packages from the local nuget cache (most of the cases did not work because of file locking issues).

I have made a PR to address this (#77).

This PR uses a different approach: it reconfigures nuget for the test projects to use an alternative local nuget cache folder somewhere in $TEMP, so that the execution always starts with an empty nuget cache. (There is a NuGet/Home#5619 in nuget where this idea was mentioned.) The nuget cache has several megabytes, so instead of making a local nuget cache for every sample project, we introduce a "Test Run" folder, that contains all sample project folders and the local nuget cache. Each time a new test run folder is created in $TEMP, but we have a cleanup mechanism that removes old test run folders.

I have tested that by replaying the changes of #54 and it works fine, all changes of the code are immediately reflected in tests.

@reqnroll/core-team: Please send a few 👍 on the PR or here if you are OK with this or make your comment to the PR if not. Would like to merge this tomorrow if there is no disagreement.

[gasparnagy]

merged