Closed obligaron closed 2 months ago
That is cool @obligaron. I was also thinking about something similar. Three questions:
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.I'm glad you like the idea. π
Regarding your questions
SkippableTestMethodAttribute' to mimic the behaviour of the
SkippableFactAttribute'. Is this a good way to do this?SkippableTestMethodAttribute
approach.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).@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.
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. π
@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
}
The test is now done in the UnitTest. π Please take a look.
@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.
Thanks @gasparnagy π
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) ![grafik](https://github.com/reqnroll/Reqnroll/assets/25415264/d8815a4f-e75c-497a-8473-d234b47410d3) #### After (PR) ![grafik](https://github.com/reqnroll/Reqnroll/assets/25415264/103e5623-aa03-498d-be99-f2581e612487) ### Test GeneratorAllIn_sample_can_be_handled #### Before (main) ![grafik](https://github.com/reqnroll/Reqnroll/assets/25415264/73d9a794-bc23-4acc-9d48-c8e8660e5cf1) #### After (PR) ![grafik](https://github.com/reqnroll/Reqnroll/assets/25415264/992446ca-667b-45ce-9500-62cc54fe5c55)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
Checklist: