[Opensim-dev] Unit tests should not be coupled (WAS: Re: [Opensim-commits] r8737 - trunk/OpenSim/Framework/Communications/Tests)

[Stefan Andersson]

And while on the subject, tests invoking the database layer are not unit tests, they are integration tests, and should be run as a separate batch. Unit tests should be FAST so you can run the whole suit on each refactoring without hesitation.

Best regards,
Stefan Andersson
Tribal Media AB



 
> Date: Tue, 10 Mar 2009 18:24:56 +0900
> From: mmazur at gmail.com
> To: mmazur at gmail.com
> CC: arthursv at linux.vnet.ibm.com; opensim-dev at lists.berlios.de
> Subject: Re: [Opensim-dev] Unit tests should not be coupled (WAS: Re: [Opensim-commits] r8737 - trunk/OpenSim/Framework/Communications/Tests)
> 
> Hi,
> 
> On Tue, 10 Mar 2009 17:03:44 +0900
> Mike Mazur <mmazur at gmail.com> wrote:
> 
> > I'm pretty sure by using
> > fixtures properly in OpenSim's unit tests, the need for Chained Tests
> > can be eliminated.
> 
> I took a look at OpenSim/Data/Tests/BasicGridTest.cs. There are a few
> tests there that depend on the order they're executed in order to
> succeed. By removing the T???_ prefix from the methods, the unit tests
> fail:
> 
> Test Case Failures:
> 1) OpenSim.Data.MySQL.Tests.MySQLGridTest.LoadEmpty : Expected: null
> But was: <OpenSim.Data.RegionProfileData>
> 
> at OpenSim.Data.Tests.BasicGridTest.LoadEmpty () [0x00000]
> at (wrapper managed-to-native)
> System.Reflection.MonoMethod:InternalInvoke
> (object,object[],System.Exception&) at
> System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags
> invokeAttr, System.Reflection.Binder binder, System.Object[]
> parameters, System.Globalization.CultureInfo culture) [0x00000]
> 
> 2) OpenSim.Data.MySQL.Tests.MySQLGridTest.RegionList : Expected: 2
> But was: 1
> 
> at OpenSim.Data.Tests.BasicGridTest.RegionList () [0x00000]
> at (wrapper managed-to-native)
> System.Reflection.MonoMethod:InternalInvoke
> (object,object[],System.Exception&) at
> System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags
> invokeAttr, System.Reflection.Binder binder, System.Object[]
> parameters, System.Globalization.CultureInfo culture) [0x00000]
> 
> This is expected. So I refactored those tests a little in order to get
> rid of this dependency. I'm attaching the result to this email; look at
> this email in the archives[1] to download the attachment.
> 
> Here's a summary of the changes:
> 
> - SimpleAddRetrieveProfile() test can be removed, as
> AddRetrieveCompleteTest() tests the same code
> - RandomName() looks like a utility function and doesn't have any
> asserts, so it doesn't need the [Test] attribute
> - extracted the region adding code in AddRetrieveCompleteTest() into a
> method called createRegion(UUID, string) which creates a region in
> the database; this method is called from any test that needs a region
> to exist in the DB
> - added a [TearDown] method, which is called after every test has been
> executed by the NUnit framework, which removes all the regions from
> the DB
> 
> If nobody has any strong objections to this approach to isolating
> tests, I'll commit this tomorrow. Also, it's probably a good idea to
> refactor other tests in a similar fashion. If chaining tests is not
> absolutely necessary, it shouldn't be done.
> 
> One more thing to note, those tests use GridDataBase.AddProfile() to add
> regions to the DB and GridDataBase.GetProfile*() to get the region back.
> This is flawed; imagine the GridDataBase implementation had a bug that
> prevented anything from actually being stored to disk. Its caching
> mechanism worked as expected, though, and anything that was added with
> AddProfile() was always available with GetProfile*() as long as the
> application (grid server) was running. The functionality of actually
> reading the disk/DB was never actually tested.
> 
> To ensure the correct functionality is being tested, one would insert
> the data into the DB by different means (possibly SQL statements in the
> tests), then retrieve them using the proper GetProfile*() method.
> 
> Thanks,
> Mike
> 
> 
> [1]
> https://lists.berlios.de/pipermail/opensim-dev/2009-March/thread.html
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://opensimulator.org/pipermail/opensim-dev/attachments/20090310/e522401e/attachment-0001.html>