bblanchon / WpfBindingErrors

:boom: Turn WPF Binding errors into exception
MIT License
78 stars 17 forks source link

switch to XUnit #10

Open TysonMN opened 4 years ago

TysonMN commented 4 years ago

I was not able to run the tests in Visual Studio (Edition: Enterprise 2019, Version: 16.7.2). I got them to run after switching to XUnit and using XUnit.StaFact.

P.S. I am very grateful for your project. I wanted to add a test that fails if there are any binding errors and your use of... https://github.com/bblanchon/WpfBindingErrors/blob/f9152afc53b12fe693008880cf840ad6a8fea87b/WpfBindingErrors/BindingErrorListener.cs#L23

...seems necessary. I couldn't any solution without that line. Furthermore, in all the other sources that I encountered, I didn't see anyone else use that line.

I don't understand how that line helps.

bblanchon commented 4 years ago

Hi @bender2k14,

Thank you very much for this contribution.

Unfortunately, I've been out of the C# game for six years, and my chances of getting back to WPF diminish every day. I don't remember why this line is here, and I'm afraid that removing it could break something.

Regarding the change to XUnit, I'm assuming MSTest is not available anymore, and that's why you replaced it. However, I'd really like a second opinion before merging this PR because, again, I don't want to break anything.

Best regards, Benoit

TysonMN commented 4 years ago

Your hesitation to merge is good. I don't want you to merge unless you are confident of the change. Whatever decision you make (merge, reject, or leave open "forever") is fine with me. My branch will continue to exist in my fork of your repo, and this PR will continue to link to it. In that way, people that visit your repo still have a good chance of find and (potentially) benefiting from my change suggestion.

My (long term) plan is to write a blog post about such a test. When that post exists, I will come back here and share a link to it.

sa-he commented 4 years ago

MSTest or xUnit (to me) is mostly a matter of taste. So switching xUnit can be ok.

I used WpfBindingErrors for a while, but had more and more false-positive warnings. As I learned that Microsoft is developing an experimental extension that eventually might end up in an official Visual Studio version, I chose that. This extension does not throw exceptions, but list binding errors in a window. So false-positives are not as annoying.

https://marketplace.visualstudio.com/items?itemName=PeterSpa.XamlBinding

TysonMN commented 4 years ago

Ah, I forgot to comment about MSTest vs XUnit. No, MSTest is still available. In the past, MSTest was lagging in features compared to NUnit and XUnit. I don't know how feature sets compare today.

I tried to get your code to work for me using MSTest, but I was unable to do so. Instead, I was able to get it working using XUnit, and that is the code in my branch.

I used WpfBindingErrors for a while, but had more and more false-positive warnings.

Interesting. Are you able to provide an example that shows a false positive?

As I learned that Microsoft is developing an experimental extension that eventually might end up in an official Visual Studio version, I chose that. This extension does not throw exceptions, but list binding errors in a window. So false-positives are not as annoying.

https://marketplace.visualstudio.com/items?itemName=PeterSpa.XamlBinding

Thanks for the link! This is great to know.