Misc improvements to ExtensionServiceTests #454
Conversation
| } | ||
|
|
||
| Assert.Fail("Couldn't find known Extension {0}", typeName); | ||
| Assert.That(_serviceInterface.Extensions, |
There was a problem hiding this comment.
Before we iterated over _serviceClass.Extensions. I don't know much about this area, so why this change?
There was a problem hiding this comment.
The main aim of this was just to get a better message in CI when the test fails.
The old version failed without telling you what was actually in the collection. The new version prints what the collection currently is - allowing you to have a better idea of what caused the problem. 🙂
There was a problem hiding this comment.
In all the refactorings you did in this PR you "translated" a foreach loop, e.g. foreach (IExtensionNode node in _serviceInterface.Extensions) into an assertion over the same collection Assert.That(_serviceInterface.Extensions, ....
But in this test the foreach loop was foreach (IExtensionNode node in _serviceClass.Extensions), but this was translated into Assert.That(_serviceInterface.Extensions, ..., so I was wondering why the change from _serviceClass.Extensions to _serviceInterface.Extensions.
There was a problem hiding this comment.
Damn - you're right - sorry, I missed what you were getting at. Not intentional - good catch!
These tests caused me some debugging issues during #452, as they were failing on Mono only.
I've switched them over to use the newly-improved CollectionConstraint, which gives us more detailed error messages when they fail, in terms of what the collection actually looked like. I also tidied formatting/naming-conventions etc. while I was there.