[WiP] - Repair & Update Unit Tests

Discussion in 'Submit: code patches (MediaPortal/TV-Server/etc.)' started by Tony Wall, May 1, 2014.

  1. Tony Wall
    • Premium Supporter

    Tony Wall MP Donator

    Joined:
    May 31, 2012
    Messages:
    18
    Likes Received:
    9
    Gender:
    Male
    Ratings:
    +14 / 0
    Home Country:
    United Kingdom United Kingdom
    In Media Portal 1 the unit tests are clearly not being used which makes development difficult and risky.

    Problem:
    1) They are still configured to build with the .NET Framework 3.5 which doesn't work because the rest of the system was already upgraded to .NET 4.
    2) They don't compile for various other reasons.
    3) They are dependent on a third party tool which is closed source and has no direct download link, i.e. Typemock.
    4) They have clearly been set aside because they are hidden in a different solution which does not build and large portions of the code are commented out.
    5) No evidence of any Test Driven Development because of the above, especially #4. Unit tests should be in the same solution as the code, ready to use, integrated with Visual Studio.

    Background / Justification:
    So I thought I finished a plugin patch which operates on external data and want to test it, but there is nowhere for me to add this test. Worse, it looks like nobody does any testing at all! In this case especially, I must work with a complex and sparsely documented component, WebEPG, with most debug information either missing or deliberately disabled in the code! The complexity of the templates and regular expressions one must build to extract the data make it overly time consuming and very frustrating. When a unit test existed it would be a quick case of stepping through targeted code (without the whole system built and installed) with the debugger to get straight to the cause.

    Goals:
    1) Restore the unit tests to the solutions using the current testing frameworks.
    2) Remove third party closed source dependencies, i.e. Typemock. This is especially annoying since they have no NuGet package and do not offer a direct download link (really pushing you towards the free trial of the paid version, requiring your personal data).
    3) Re-activate as many of the existing/forgotten unit tests. Remove code which is depreciated.
    4) From this baseline, as a proof of concept, finally add my unit test in the proper location (what should have been easy in the first place).
    5) Update documentation/wiki/standards/forum to make it clear of their existence and encourage quality/TDD.



    So far it looks like XUnit.NET (NUnit replacement available via NuGet) should be used with some kind of auto-mocking extension(s) (to replace Typemock) such as AutoFixture with NSubstitute or Rhino Mocks, etc... basically whatever fits best with current .NET and Visual Studio versions, is popular, open source or free from Microsoft, maintained and can be used/built without complexity or obscurity.

    I've already got plenty of XUnit experience and how to migrate to it so that'll be done quickly. I'll start with the TVLibrary solution so I can get my WebEPG patch tested and submitted. Right now I'm researching the best Typemock replacement which is the only delay, but should be done carefully (suggestions welcome).
     
    • Like Like x 2
    • Thank You! Thank You! x 2
  2. Google AdSense Guest Advertisement



    to hide all adverts.
  3. FreakyJ
    • Team MediaPortal

    FreakyJ Development Group

    Joined:
    July 25, 2010
    Messages:
    4,021
    Likes Received:
    839
    Gender:
    Male
    Ratings:
    +1,424 / 1
    Home Country:
    Germany Germany
    You should consider doing this for TVE 3.5 :)
     
    • Agree Agree x 1
  4. Tony Wall
    • Premium Supporter

    Tony Wall MP Donator

    Joined:
    May 31, 2012
    Messages:
    18
    Likes Received:
    9
    Gender:
    Male
    Ratings:
    +14 / 0
    Home Country:
    United Kingdom United Kingdom
    This post helps understand which mock framework is better when searching for a Typemock replacement (specifically best support for "moles (shims)":

    http://stackoverflow.com/questions/9677445/mock-framework-vs-ms-fakes-frameworks

    Typemock seems like it was one of a kind in the past, so an understandable choice (then). Looking for an MS standard leads to their Fakes Framework which unfortunately is now a premium product (available only in Visual Studio Premium or Ultimate editions).

    Jim Cooper makes some good points (in that Stack Overflow post) against NMock, along with examples. So that rules itself out too. Looks like it's down to Rhino Mocks or Moq (or something else I find on the way).

    Searching...
     
    • Like Like x 1
  5. Tony Wall
    • Premium Supporter

    Tony Wall MP Donator

    Joined:
    May 31, 2012
    Messages:
    18
    Likes Received:
    9
    Gender:
    Male
    Ratings:
    +14 / 0
    Home Country:
    United Kingdom United Kingdom
    Okay I think I've read enough now to invest time migrating to XUnit.NET, Moq and AutoFixture (including it's own Moq extension). Mocking test data will be provided by AutoFixture and interception/isolation of dependencies (a.k.a. shims/moles/dynamic-proxies) is Moq's thing.

    In my opinion the choice between Moq and RhinoMocks is won by Moq mainly because of it's simpler coding style but also because it appears to be more of a pure .NET open source project whereas the hibernatingrhinos.com (RhinoMock) web site clearly has a commercial product first approach (no better than Typemock then). There was another called FakeItEasy which has a simple and fun approach but I get the feeling that Moq is more professional.

    @FreakyJ - Yes I want to propagate this to all the solutions. Hopefully it's straightforward with experience gained from the first conversion. If not for others at least for my own confidence that I am building everything correctly (the internals appear to be working) before submitting patches. I hope the main developers agree and don't just reject my changes :)
     
  6. FreakyJ
    • Team MediaPortal

    FreakyJ Development Group

    Joined:
    July 25, 2010
    Messages:
    4,021
    Likes Received:
    839
    Gender:
    Male
    Ratings:
    +1,424 / 1
    Home Country:
    Germany Germany
  7. FreakyJ
    • Team MediaPortal

    FreakyJ Development Group

    Joined:
    July 25, 2010
    Messages:
    4,021
    Likes Received:
    839
    Gender:
    Male
    Ratings:
    +1,424 / 1
    Home Country:
    Germany Germany
    Ahh and @morpheus_xx
    Didn't you started to implement unit tests into MP2?! Maybe we should stay with one approach?
     
  8. mnmr
    • Premium Supporter

    mnmr Retired Team Member

    Joined:
    December 27, 2008
    Messages:
    180
    Likes Received:
    59
    Occupation:
    Software Developer
    Location:
    Copenhagen
    Ratings:
    +101 / 0
    Home Country:
    Denmark Denmark
    I would recommend that you take a look at NSubstitute before settling on Moq. It has superior syntax (much more readable) and you can find many articles on Google from people explaining how to migrate from Moq to NSubstitute. One example here (shows a few examples side-by-side for the two frameworks)

    It should be noted that TypeMock uses the profiler API, which allows it to mock everything that happens - including intercepting object creation or access to non-virtual members or methods. This is not supported by any other mocking framework I know of. Thus, without TypeMock, your code needs to be "designed for unit testing". This means that classes must provide a means of injecting dependencies - typically in the form of a constructor where dependencies can be passed, but sometimes also by a writable property (I prefer to use constructor arguments for required dependencies and properties for optional ones).

    This is not the case with the current MP2 source code, so if you are serious about improving the unit test situation, I really think that one of the top priorities should be to refactor classes to be unit testable. Basically, every call to "new" (object creation) or "ServiceRegistration.Get" (object lookup) needs to be replaced with a constructor parameter (if the type is an MP2 type; it is often fine to create BCL types). Without this, no benefit comes from introducing a new unit test libraries, as no one will be able to use them to test the code.

    I've heard complaints that you cannot reliably cache instances retrieved from ServiceRegistration, as they might get replaced while the program is running. This sounds really weird and reeks of bad practice (in my humble, but vocal, opinion). Injecting dependencies via constructors would amount to the same as caching instances inside classes, so this practice would need to end if MP2 is to become unit testable. Should some service really require this ability, I suggest that we create a "service proxy" (basically a wrapper around the actual service, so that the instance itself can be replaced when the wrapper determines the need to do so) and pass the proxy as a dependency instead of the service itself. This would also have the benefit of making the behavior explicit (as in, "this dependency uses a proxy so it is not a true singleton") rather than the current "I don't know if anybody uses it, but technically it could happen" situation.

    These are my two cents, anyway :)
     
    • Thank You! Thank You! x 1
    • Agree Agree x 1
  9. chefkoch
    • Premium Supporter

    chefkoch Retired Team Member

    Joined:
    October 5, 2004
    Messages:
    3,130
    Likes Received:
    1,456
    Gender:
    Male
    Location:
    Dresden / Munich / Maastricht
    Ratings:
    +1,773 / 1
    Home Country:
    Germany Germany
    I have not that much experience how to refactor existing tests from one framework to the other, which framework has pro and cons etc...
    But what I would like see is using a framework, (nunit or mstest)..., which requires almost nothing to setup when actually compiling and testing the code. So if it is possible to integrate it via NuGet, perfect.

    Regarding the code you want to refactor: I would understand if you want to work on the code, which is currently running on your MP1 installations (TVE3).
    Nevertheless the tvlibrary has been completely restructured within the last years. Current code is available here:
    https://github.com/morpheusxx/MediaPortal-1
    https://github.com/MediaPortal/MediaPortal-1/tree/EXP-TVE3.5-MP1-MP2
    Just for your information to prevent any surprise when finding TVE35.
     
    • Informative Informative x 1
  10. Tony Wall
    • Premium Supporter

    Tony Wall MP Donator

    Joined:
    May 31, 2012
    Messages:
    18
    Likes Received:
    9
    Gender:
    Male
    Ratings:
    +14 / 0
    Home Country:
    United Kingdom United Kingdom
    Been on holiday... in response to the various points:

    Moq vs. NSubstitute
    @mnmr I agree, on further examination although they seem similar the following post confirms at least one good reason to choose NSubstitute over Moq.
    http://dennis-nerush.blogspot.de/2012/10/rhino-mocks-vs-moq-vs-nsubstitute.html
    AutoFixture also supports NSubstitute so that's okay too.
    Switching from one tool to the other is no big issue, they usually follow the same pattern. Having any kind of asserted "fact" (test) in any decent form of code is most important.

    Frameworks
    I'm not one of those people who uses a framework for a the sake of it. I'm more of an old school component and object orientated developer who tries to make sound design decisions first leading to a more elegant object model not requiring bloating or overhead of additional runtime frameworks. I know what I'm talking about. I've used the Patterns & Practices of Microsoft right from the start. The P&P team themselves advise their frameworks/extensions usually end-up in a more compact/concise form in the next version of the .NET Framework itself. Dependency Injection and Log4Net (the competitor to Logging Application Block from MANY years ago) are simply not required in most (if not all sensible) cases. Just consistently use good design and built-in features, i.e.
    * Component architecture
    * Interfaces where extension is supported
    * A simple factory pattern only where needed
    * Trace sources with logical names/splits
    * Good code documentation and clear log messages
    * Standard binding redirects for versioning
    * Strong names to support GAC and isolate processor specific limitations
    MP1 fails quite a bit here and is a legacy product so I wouldn't even try to do too much here. Maybe I could bring something positive to MP2 later.

    Scope
    My initial goal is to be able to unit test patches in MP1, to cover some essential core features I feel are not working right (at least for me) and to provide a place for others to put some new tests in if they want to. The only framework switching here is replacement of 3rd party or legacy components with NuGet open source alternatives so the build works on every developer PC without external fiddling/installation/inconsistency. I will check out MP2 source to make sure I'm not going against a previous decision, if there is one/evidence/code which covers these concerns, then start a new discussion about that as necessary. That should not delay submission of a properly tested MP1 patch though.
     
    • Like Like x 3
Loading...

Users Viewing Thread (Users: 0, Guests: 0)

  1. This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
    By continuing to use this site, you are consenting to our use of cookies.
    Dismiss Notice
  • About The Project

    The vision of the MediaPortal project is to create a free open source media centre application, which supports all advanced media centre functions, and is accessible to all Windows users.

    In reaching this goal we are working every day to make sure our software is one of the best.

             

  • Support MediaPortal!

    The team works very hard to make sure the community is running the best HTPC-software. We give away MediaPortal for free but hosting and software is not for us.

    Care to support our work with a few bucks? We'd really appreciate it!