UnitTests: Check if SDK version is installed and if not ignore the test

[obligaron]

This PR changes the tests to be ignored/skipped, if the required SDKs/TargetFrameworks are not installed on the local machine.
This helps to improve the (first time) developer experience (at least for me 😉).

Before/After changes

Test CreateEmptyCSharpCore3_1ProjectInNewFormat

Before (main)

After (PR)

Test GeneratorAllIn_sample_can_be_handled

Before (main)

After (PR)

With this PR no tests fail on my machine (windows) even if not all SDKs/TargetFrameworks are installed.

If desired, we can disable this behaviour when running tests in CI to ensure that all relevant SDKs and frameworks are installed. (Currently not implemented.)

Types of changes

• 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 not work as expected).
• Performance improvement
• Refactoring (so no functional change)
• Other (docs, build config, etc)

Checklist:

• I've added tests for my code. (most of the time mandatory)
• I have added an entry to the changelog. (mandatory)
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[gasparnagy]

That is cool @obligaron. I was also thinking about something similar. Three questions:

1. Isn't there an option in SystemTests (that uses MsTest) to check the exception that was thrown and do the dynamic ignore in the TestCleanup? (So that we don't have to do the try-catch on the tests) Maybe this info is in the TestContext somewhere...
2. If that is not possible, maybe we can provide a method in the base class, that takes an Action parameter and wraps it with the try-catch. So that if we change that logic, we don't need to do it for each case.
3. The ConfigurationDriver class provides a setting called PipelineMode, that is true when the tests run in the pipeline. Could we change the SystemTests in a way that it only does the dynamic ignore if it is running locally (so the PipelineMode is false). I fear that if something changes on our CI agent, we would not be modified about an untested framework.

[obligaron]

I'm glad you like the idea. 🙂

Regarding your questions

1. I added a new SkippableTestMethodAttribute' to mimic the behaviour of the SkippableFactAttribute'. Is this a good way to do this?
2. I can also go this way if you don't like the SkippableTestMethodAttribute approach.
3. I added the PiplineMode check for CompilationDriver related errors (last commit).
   But I wasn't sure how to access the property in SolutionWriter'. I couldn't find a member that contained a ConfigurationDriver. What would be a good way to access the information? So currently the PiplineMode` check is only half implemented (which is why I changed the PR to draft).

[gasparnagy]

@obligaron this is all good. I did not know that you can make such fancy stuff with the TestMethodAttribute.

For the pipeline mode: My original idea was to add the condition to the SkippableXXX attribute, so you only change the outcome if no pipeline mode (maybe there will be other similar checks). There you can just create a new instance of the ConfigurationDriver there (and get the PipelineMode property), it does not have a dependency and it is cheap to create one.

If that does not work or if you prefer to do the checks in the TestProjectGenerator, you can do the check in the SolutionWriter (or any other class there, like the CompilationDriver), just simply add the ConfigurationDriver to the ctor and the DI will resolve it. You don't have to route it through the other classes.

[obligaron]

I added ConfigurationDriver to SolutionWriter. 😉

But I would also be fine with doing the check in the test methods instead when the throw happens.
The only down-side is that I think we can't parametersize SkippableFact dynamically. We would need a Method that takes a action parameter and do the check (like you suggested in your first comment). If you prefer it that way I can change the PR to this approach. 🙂

[gasparnagy]

@obligaron Thx.

You do not need to really make the SkippableFact parameterizable for this. The ConfigurationDriver is anyway reading this configuration from environment variables. So anywhere in the SkippableFact you can write things like:

if (! new ConfigurationDriver().PipelineMode) {
  //do the skipping
}

[obligaron]

The test is now done in the UnitTest. 😉
Please take a look.

[gasparnagy]

Looks good!

[gasparnagy]

@obligaron Thx. I have invited you to the contributors team (you need to accept the invitation). Being in this team means that the CI build will automatically run for your PRs and that you can create new branches in the main repo. This latter means that if you would like to contribute with a change, you don't need to fork the Reqnroll project (or sync your fork), but you can simply clone the main repository, create a new branch, push that up and create a PR for that branch. This also has the benefits, that others can help you with code changes in your PR just by adding commits to that branch.

[obligaron]

Thanks @gasparnagy 🙂