Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,539

filter by tags archive

Test refacotring


I just posted about a horribly complicated test, I thought I might as well share the results of its refactoring:

[TestFixture]
public class IndexedEmbeddedAndCollections : SearchTestCase
{
	private Author a;
	private Author a2;
	private Author a3;
	private Author a4;
	private Order o;
	private Order o2;
	private Product p1;
	private Product p2;
	private ISession s;
	private ITransaction tx;

	protected override IList Mappings
	{
		get
		{
			return new string[]
			{
				"Embedded.Tower.hbm.xml",
				"Embedded.Address.hbm.xml",
				"Embedded.Product.hbm.xml",
				"Embedded.Order.hbm.xml",
				"Embedded.Author.hbm.xml",
				"Embedded.Country.hbm.xml"
			};
		}
	}

	protected override void OnSetUp()
	{
		base.OnSetUp();

		a = new Author();
		a.Name = "Voltaire";
		a2 = new Author();
		a2.Name = "Victor Hugo";
		a3 = new Author();
		a3.Name = "Moliere";
		a4 = new Author();
		a4.Name = "Proust";

		o = new Order();
		o.OrderNumber = "ACVBNM";

		o2 = new Order();
		o2.OrderNumber = "ZERTYD";

		p1 = new Product();
		p1.Name = "Candide";
		p1.Authors.Add(a);
		p1.Authors.Add(a2); //be creative

		p2 = new Product();
		p2.Name = "Le malade imaginaire";
		p2.Authors.Add(a3);
		p2.Orders.Add("Emmanuel", o);
		p2.Orders.Add("Gavin", o2);


		s = OpenSession();
		tx = s.BeginTransaction();
		s.Persist(a);
		s.Persist(a2);
		s.Persist(a3);
		s.Persist(a4);
		s.Persist(o);
		s.Persist(o2);
		s.Persist(p1);
		s.Persist(p2);
		tx.Commit();

		tx = s.BeginTransaction();

		s.Clear();
	}

	protected override void OnTearDown()
	{
		// Tidy up
		s.Delete("from System.Object");

		tx.Commit();

		s.Close();

		base.OnTearDown();
	}

	[Test]
	public void CanLookupEntityByValueOfEmbeddedSetValues()
	{
		IFullTextSession session = Search.CreateFullTextSession(s);

		QueryParser parser = new MultiFieldQueryParser(new string[] { "name", "authors.name" }, new StandardAnalyzer());

		Lucene.Net.Search.Query query = parser.Parse("Hugo");
		IList result = session.CreateFullTextQuery(query).List();
		Assert.AreEqual(1, result.Count, "collection of embedded (set) ignored");
	}

	[Test]
	public void CanLookupEntityByValueOfEmbeddedDictionaryValue()
	{
		IFullTextSession session = Search.CreateFullTextSession(s);
		
		//PhraseQuery
		TermQuery  query = new TermQuery(new Term("orders.orderNumber", "ZERTYD"));
		IList result = session.CreateFullTextQuery(query).List();
		Assert.AreEqual(1, result.Count, "collection of untokenized ignored");
		query = new TermQuery(new Term("orders.orderNumber", "ACVBNM"));
		result = session.CreateFullTextQuery(query).List();
		Assert.AreEqual(1, result.Count, "collection of untokenized ignored");
	}

	[Test]
	[Ignore]
	public void CanLookupEntityByUpdatedValueInSet()
	{
		Product p = s.Get<Product>(p1.Id);
		p.Authors.Add(s.Get<Author>(a4.Id));
		tx.Commit();

		QueryParser parser = new MultiFieldQueryParser(new string[] { "name", "authors.name" }, new StandardAnalyzer());
		IFullTextSession session = Search.CreateFullTextSession(s);
		Query query = parser.Parse("Proust");
		IList result = session.CreateFullTextQuery(query).List();
		//HSEARCH-56
		Assert.AreEqual(1, result.Count, "update of collection of embedded ignored");

	}
}

It is almost the same as before, the changed are mainly structural, but it is so much easier to read, understand and debug.


Comments

John

Instead of cleaning up using deletes, why don't you just rollback the transaction?

John

And replacing tx.Commit with s.Flush(); s.Clear(); in your tests

Benny Thomas

I'm missing the factory method for Author, Order and Product.

Ayende Rahien

John,

What you don't see here is that there are transaction syncronizations going on behind the scene to sync the index.

I have to use tx.

Ayende Rahien

Benny,

Not worth it, this is not a test for an application, this is a test for NH Search, so we have LOTS of stuff like that

Jonathan Allen

So instead of a single test you can easily follow you have a tangle of implied function calls.

And what if one of those implied calls, say "OnSetUp" fails? Do you get bombarded with false positives or does it just silently ignore your tests?

And what if you want a different setup? Do you have to build a whole new class for each data scenario?

The original test wasn't perfect, but this new one seems to be both over engineered and inflexible at the same time.

Benny Thomas

@Ayende: I know it's not worth it, but it is good practice. And this is codesmell and people who wan't to learn what happens behind the scene will get a nearly Kobe feeling ;)

The power of example.

Frank Quednau

Finally a prominent proponent for PascalCasedTestNames. Allthisunderscoreseparationisgettingoutofhand! ;)

Victor Kornov

@Frank Quednau

I'd say that's naming convention in NH which Oren just follows.

John Simons

Not sure if it's intentional or not but you may want to fix the spelling mistake "Refactoring" Cheers John

Ciaran Jessup

I notice in your OnTearDown you're deleting every mapped thing in your NHibernate Configuration, which means (unless I'm mistaken, which I could well be) that if there was any data in there that was global reference data that was shared by all tests it would be blown away.

I've always been told to strive for Idempotence in my tests so would traditonally have just removed what I created in my setup, am I wasting my time with this approach ?

In a non-trivial test where you can setup the entire database as you need it in your setup then I guess its ok to do this each time, but when you're running hundreds or thousands of tests over a complex system that has data that has a lot of shared reference values that would slow the tests down quite a lot I think ?

Ayende Rahien

Ciaran,

The lack of context is tripping you.

In fact, I am blowing away the database schema each time I run the test.

Ciaran Jessup

Interesting, does this not slow down your tests a lot ?

Ayende Rahien

Ciaran,

a) no

b) you haven't asked what I am testing, this is part of the NH Search test suite

Ciaran Jessup

I didn't mean to cause offence, was just interested! And i'll take your word for it then :)

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

  1. RavenDB Sharding - Adding a new shard to an existing cluster, splitting the shard - 30 minutes from now

There are posts all the way to May 22, 2015

RECENT SERIES

  1. RavenDB Sharding (2):
    21 May 2015 - Adding a new shard to an existing cluster, the easy way
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats