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

[Mike Mazur]

Hi,

On Tue, 10 Mar 2009 17:10:42 -0300
Arthur Valadares <arthursv at linux.vnet.ibm.com> wrote:

> Still some tests must be chained (you can't test an Update without a
> Create for example). And for that reason, and also to keep the logic
> of whoever reads the test, I feel we should still keep writing tests
> with some kind of logical order. 

An update can definitely be tested without testing create first. To test
the update, the data would be inserted into the DB by another means
first. For example:

// pseudocode
[TestFixture]
public class MyTest
{
  UUID m_regionUUID = <some UUID>;

  [SetUp]
  public void SetUp()
  {
    CreateNewDB("OpenSim");
    sql.execute("INSERT INTO regions VALUES('My Region',m_regionUUID)");
  }

  [TearDown]
  public void TearDown()
  {
    DropDB("OpenSim");
  }

  [Test]
  public void TestCreate()
  {
    UUID thisUUID = <another UUID>;

    object_under_test.createRegion("New Region", thisUUID)

    Region region = sql.execute("SELECT FROM regions WHERE
    uuid=thisUUID");
    assertEquals(region.UUID, thisUUID);
    assertEquals(region.Name, "New Region");
  }

  [Test]
  public void TestUpdate()
  {
    Region updatedRegion = new Region();
    updatedRegion.UUID = m_regionUUID;
    updatedRegion.Name = "Updated region";

    object_under_test.updateRegion(m_regionUUID, updatedRegion);

    Region region = sql.execute("SELECT FROM regions WHERE
    uuid=m_regionUUID");
    assertEquals(region.UUID, m_regionUUID);
    assertEquals(region.Name, "Updated Region");
  }
}

I'm actually trying to show two things here:

1. The SetUp and TearDown methods are called by the NUnit framework
   before and after each test is executed, repsectively. They ensure
   that each and every single test starts with the exact same data
   populated in the DB. This means each test can be run independently
   of any other test and doesn't depend on any other test being
   run/succeeding/failing.

2. The object under test is only used in the tests. In the SetUp,
   TearDown, and even verification code, another (known to work) method
   is used to interact with the database (in the example, it's raw
   SQL). In BasicGridTest.cs I noticed that the object under test
   (the GridDataBase class) is being tested, but it's also used to
   populate the database, and to verify that the data was indeed
   written to the database/disk. This is not good, as I mentioned in
   another email[1].

> This will also help debugging some unit tests where you modify
> something but has no knowledge that this modification changes your
> "expected initial status" of other tests. There are lot's of obscure
> relations in OpenSim and a test creator might fail to see the whole
> picture.

If a developer makes a change that affects "expected initial status" of
many other tests, this is a warning sign that the developer is changing
something of importance. If this change is indeed necessary, then all
the tests that are affected should be modified to reflect the new state
of the source code. If fixtures (SetUp, TearDown) are used properly,
the changes will likely be limited to them.

> In summary, I don't see a reason not to organize the tests with TXXX.
> I'll keep working them that way, but this is another one of those
> discussions with no right and wrong, just hunches of what's more
> efficient. :)

Sure.

Mike


[1]
https://lists.berlios.de/pipermail/opensim-dev/2009-March/005625.html