search

Fody / PropertyChanged

Injects INotifyPropertyChanged code into properties at compile time
MIT License
1.88k stars 227 forks source link

Weaving not applied during Live Unit Testing #776

Closed richardhauer closed 2 years ago

richardhauer commented 2 years ago

Bit new to Fody so pls go easy. There are a few moving parts to this that I've tried as a result of reading various posts/articles. I'm unsure if this is an issue with PropertyChanged.Fody or if Fody in its entirety is not being executed under LUT scenarios. Happy to post to another location if that's appropriate but thought I'd start at the highest level of specificity.

Environment

I have a basic soln structure as follows:

<PackageReference Include="Fody" Version="6.5.3">
    <PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="PropertyChanged.Fody" Version="4.0.0">
    <PrivateAssets>all</PrivateAssets>
</PackageReference>

Problem

Nothing I can do will make OnPropertyChanged() fire in the context of Live Unit Testing. I can get it to work when running through Test Explorer and via dotnet test so this isn't an issue with the normal operation of the library per se.

I have been unable to identify a discrepancy between the build process used in LUT/roslyn and the normal dotnet build/msbuild so I can't imagine why it wouldn't work, hoping someone with experience in this area can direct me to whatever changes might be necessary to get LUT going.

Observation: Fody & PropertyChanged.Fody packages need to be applied to both the library project AND the MSTest project in order for it to work through Test Explorer - I would not have expected this to be the case, but may be a clue. Presumably these references are only required on the project that actually uses the weaving?

Anyone have any suggestions or thoughts?

richardhauer commented 2 years ago

A PS for anyone stumbling over this with the same issue. It's possible to disable execution of a test in the context of LUT while still executing it as part of a Build pipeline or in Test Explorer.

Just apply the attribute [TestCategory("SkipWhenLiveUnitTesting")] to your test.

ltrzesniewski commented 2 years ago

As you're developing an application, Live Unit Testing automatically runs any impacted unit tests in the background and presents the results and code coverage in real time.

I'm not really surprised it doesn't work, since VS needs to do coverage analysis...

We could try to hook Fody on $(TargetsTriggeredByCompilation) to see if it works better that way (currently it is being hooked with AfterTargets="AfterCompile").

Fody & PropertyChanged.Fody packages need to be applied to both the library project AND the MSTest project in order for it to work through Test Explorer

That's strange... these packages shouldn't be needed in the test project. I just tested the Test Explorer on a project of mine that uses Fody and it works (I usually use Rider instead of the Test Explorer).

richardhauer commented 2 years ago

I can create a small github repo if it helps.

After a bit more experimentation I can remove the Fody library from the Test project but I have to have the PropertyChanged.Fody there. I suppose this is because even though the base class (which is in the library project) has all the references, there is a test class that's in the Test library and the transient dependency isn't sufficient. Maybe?

In terms of the hook, I could use Directory.Build.targets to add Fody into the other target. Do you know what actions are being used? Can I locate this in the Fody repo maybe?

EDIT https://docs.microsoft.com/en-us/visualstudio/test/live-unit-testing-faq?view=vs-2019#customize-builds The sample appears to suggest that targets that are executed on AfterBuild should be ok. Looking at Fody.targets it seems like there are some conditions that might disable Fody but it's a bit hard to get logging from this part of the system to see if these conditions are causing the issue.

Condition="'$(NCrunch)' != '1' And $(FodyExecutedWeavers) != '' And $(DisableFody) != true"
richardhauer commented 2 years ago

UPDATE

Thanks @ltrzesniewski - with your direction I have been able to get this working. Two changes were needed:

1) Create a directory.build.targets file in the root of the solution. 2) Copy the FodyTarget from the Fody/Fody.targets file and make the following adjustments: a) remove the And $(DesignTimeBuild) != true clause from the Conditions attribute of the <Target /> element b) remove the DelaySign attribute from the <Fody.WeavingTask /> element

It seems the LUT is building under the "DesignTimeBuild" flag which was preventing the execution. I am not using DelaySign so this doesn't affect me at all.

I do not know whether there was a specific reason to exclude Fody execution during a DesignTimeBuild condition, or if it was simply to cater to the DelaySign; if so, I imagine we can hard-wire false on DelaySign during DesignTimeBuild which would enable LUT scenarios for everyone.

For now I'm satisfied with this outcome for my project.

tom-englert commented 2 years ago

@richardhauer

  • Test library has a class that inherits from the base and implements a basic string property (auto-implemented)

I think this could be the root cause, the class you are testing is in the test library! At least that is the reason why you need PropertyChanged.Fody in the test project!

richardhauer commented 2 years ago

@tom-englert yes that's the conclusion that I came to - the transient reference to the PropertyChanged.Fody library is not sufficient.

Coupled with the msbuild minor adjustment and I have the LUT setup working a treat now. I have just removed all my [TestCategory("SkipWhenLiveUnitTesting")] flags and all the tests are running properly with coverage showing.

tom-englert commented 2 years ago

@richardhauer would be interesting to know if that modifications is still needed if the class you are testing is NOT in the test project but in the library project.

richardhauer commented 2 years ago

Most of my tests were against classes that were not defined in the Test library. It happens that the first test was setup that way as I was testing the base class' ability to record changed properties so I can have a IsChanged( a => a.Prop ) method that would inherit into all the models I need.

I had subsequently created a number of tests against the real models, all of which are in the Library project as you'd expect, and these had tests that passed when debugged or run in Test Explorer, but failed under LUT. With my changes to the target all the tests now pass under all conditions.

richardhauer commented 2 years ago

Cross-reference: https://github.com/Fody/Fody/issues/1034

tom-englert commented 2 years ago

Fixed in Fody 6.5.5