First Attempt at adding some DocStore extensions and Test Helper stuff

[PureKrome]

S'ok.

So here's some stuff which I can't live without with my RavenDB R&D work.

DocumentStore Extensions

Mainly offering some awesomesauce power for initializing a document store with some optional seed data and some optional indexes.

Test Helpers

I'm still finding the official test helpers a bit hard core and complicated. So I've included mine. They do similar stuff but it's a -lot- easier to consume. For example, u don't have to create a Doc store and u don't need to use the using statement because the class impliments IDisposable.

There's also error checking (eg. Indexes are malformed or something).

So .. thoughts?

[mattjohnsonpint]

Yes to R# settings file and moving all projects to src. Some concerns with the other items.

TestHelpers - I think the goal should be to augment ravendb official stuff, not subvert it. I know they are just now getting the test helpers settled. My opinion would be to tweak the official test helpers project and send a pull request there. It's not really raven mainline anyway. If we're going to have a separate one in contrib, it should offer distinctly different functionality. If you were a novice user, and there are two libraries to choose, it should be crystal clear when to use one over the other. Also, it needs to target .net 4.0 since raven does.

DocumentStoreExtensions - Several good ones in here, but I'm not sure about all of the overloads. Can you use default parameter values where possible to reduce them? Also, please add xmldoc style comments so we know what they're for and why we'd want to use them. The seed data is an interesting idea, but I'm not sure it is structured generically enough. People may want to pass in all kinds of different seed data. Perhaps a method delegate (Func or Action) parameter would be better so there's some freedom of how to express the seed data?

I'm closing this pull request, send another when you have some updates. comment here if you like.

And thanks for helping!!! I do appreciate it!

[PureKrome]

Test Helpers

I thought about the same thing you're saying with regards to two seperate ones. I'm sorta hoping this might be the stepping stone to getting it into the official one. And yes, there can only be one - the H.R. one.

DocumentStoreExtensions

Heh - that's the other thing I was wondering about too. I might see if i can use defaults, also. xmldoc. bleh. necessary evil :) I wasn't going to add that until the PR is pulled (if it gets pulled). Not going to waste my time if it's rejected.

Initially I thought of also using a Func/Action .. but the problem i kept thinking about that was that at some point, i need to call RavenDb => Store(something); And I kept thinking that if people have to do that or remember to do that, etc .. it adds complexity. So I tried to simplify it => give me a collection of collections.

Lets keep the convo going here before I lock down any changes for a new PR to reduce waste.

:bang:

[mattjohnsonpint]

The other bit I'm not fond of is having a session opened for you. Users are pretty accustomed to doing that themselves and I think you'll end up with two.

One bit I saw that could be split out are some extensions for checking for server errors. I hadn't considered doing that.

I think some enhancements for index creation would be cool too, but the type catalog is overkill. You can just iterate over the types calling documentStore.ExecuteIndex for each one. That's all that's happening when the catalog is unwound anyway.

[PureKrome]

The other bit I'm not fond of is having a session opened for you. Users are pretty accustomed to doing that themselves and I think you'll end up with two.

Do you mean: in my test helper code, how I have an IDocumentSession property and that lazy creates an IDocumentStore and that the Session also impliments IDisposable. This means people don't have to create a using statement. Normally, they do/should .. and you're thinking that this is different to the norm, so it's not really a good thing?

but the type catalog is overkill

Agreed! that's why i ended up creating the Test.Helpers project because I saw that, first up and groaned.

[mattjohnsonpint]

Sometimes I specifically need multiple different sessions in the same test. And sometimes I might need an asynchronous session. Or no session at all.

Also, are you sure that if more than one test is run at the same time that they will get separate document stores? You don't want one test to interfere with another when they run in parallel.

Sorry for nixing so much. I'd like to say yes to some if it. Can you do some smaller increments? Maybe start with just the project structure changes. Then start with some helpers for checking for server errors. Work your way up to the others. I'd like to focus on the merits of each one separately. Probably hold off on test helpers for now.

Thanks. :beers:

[PureKrome]

OK. i'll start with one or two smaller ones.

Sometimes I specifically need multiple different sessions in the same test. And sometimes I might need an asynchronous session. Or no session at all.

Multiple sessions? I've ... not come across that yet (in a single test). Nor have I done async sessions. What I'm trying to target is the 80/20 rule. So after I've PR'd some tiny things, maybe we can keep chatting about that stuff, in here.

I'll come back into this thread, later.

Be kewl if u could get some of the other folks from the GGroup into this convo. (or we could take the convo to the thread in the ravendb GGroup - considering people are already there?

[mattjohnsonpint]

I made the move for the /src directory already. Seemed prudent.

Re: multiple sessions - often, I like to be explicit about separating the actions from the assertions, and I don't want to guess about session behavior (is the doc still tracked? etc.). For example:

// Act
using (var session = documentStore.OpenSession())
{
    var foo = new Foo { Id = "foos/1", Name = "Fred" };
    session.Store(foo);
    session.SaveChanges();
}

// Assert
using (var session = documentStore.OpenSession())
{
    var foo = session.Load<Foo>("foos/1");
    Assert.Equal("Fred", foo.Name);
}

By using two sessions, I can be sure that foo came back from the server - not from the session's internal cache of tracked entities. The scoping also keeps me from having to come up with another name for the foo variable.

[PureKrome]

Ok - I gotcha now :)

I'm also really blond at this stuff ... and right now all the ceremony is killing me and making it 'noisy'. I'm a fan of low-ceremony frameworks like NancyFX, etc.

So .. i've been pondering your scenario's over and over again in the last 2 or so days.. and when I saw your code this mornign when i work up - i got an idea.

So watch this space. :)

I'll re-fork and have a play and post a suggestion in here, soon. :teaser:

The best of both worlds :)

[PureKrome]

Renamed the issue header (cause my emoji's weren't working :cry: )

[PureKrome]

I come bearing gifts! :gift: :gift::gift::gift::gift::gift:

So, I've done another fork of the origin. I've then done two things.

InitializeWithDefaults

A new InitializeWithDefaults, but this is now just one method with optional args. Just like you suggested. Works great.

Two things to note.

1. I'm still passing in a list of Types for the IndexesToCreate. I tried passing in a list of AbstractIndexCreationTasks or whatever it is called, but this made it more verbose and messy for the developer (in his/her unit test) when setting things up IMO. So I think i prefer what the code looks like in the unit test -> which is what is important here IMO.
2. CreateSeed data is still a collection of anythings. I still think this is easier to grok for users than passing in funcs or actions and having to remember to all number of crazy things. KISS.

Test Helpers Contrib.

So this is back again - but just work with me here a tick. This time, i've made it so that u can have 1 session or 1000000 sessions. I've refactored the logic so the DocSessions are now a dictionary of those bad boys.

This is achived by having two things : a method and a property

1. Method: IDocumentSession DocumentSessions(key) : returns the docsession (opened) for the key.
2. Property: IDocumentSession DocumentSession : just a wrapper that calles DocumentSession(DefaultKey);

So I'll generally always use the Property .. but there's the ability to now have as many Sessions as you like.

Here's a gist showing u your example: https://gist.github.com/4299792

Not the readability and lack of ceremony ? (eg. no usings, etc).

I think this is now starting to be the best of both worlds!

I've not done a PR, but my fork is here: https://github.com/PureKrome/ravendb.contrib

No xdoc comments either. Will only PR once u're more or less happy, etc.

[PureKrome]

Hi Matt,

sorry for the delay reply .. but where are we up to with this? I've nuked my old fork, re-forked and re-added my code into the lastest version of the repo.

I've also added XML Comments to public methods/extensions.

Did u get a chance to review my gist, above? I've been using these two dll's in some work over the last month and loving it! (yes, bias is expected :blush: )

Besides one crappy unit test bug, I'd love to create a new PR.

[PureKrome]

Update: unit test now passing. All good for previewing before I create a PR.

[PureKrome]

:heart: using multiple document sessions in a single test :) (now that u've highlighted that!)

I wanted to delete a document. first (default) session does the delete.

2nd session loads the doc up .. and it's null :) so i definitely know it's really not in the doc store :)

gist: https://gist.github.com/2e78aa6d9345631b001e

[mattjohnsonpint]

Hey, sorry - I haven't had a chance to review until now, and wasn't sure if this was still in progress or if you were ready.

DocumentSessions Dictionary I think you misunderstand about how unit of work is supposed to happen. The reason I separate the sessions during testing is to ensure that you are testing clean. Given the AAA pattern, Arrange and Act can share a session, or sometimes Arrange is outside the session. But Assert should be in its own session for testing reasons only. If the first session isn't fully closed, then you aren't getting the testability you need out of the second session.

The using blocks are not just ceremony - they define the boundaries of where one session starts and the next session begins. Two simultaneous sessions makes no sense whatsoever. What happens if you end up with the same entity tracked by more than one session? I dunno, but it can't be good. What do you have against using statements anyway?

InitializeWithDefaults I'm still not liking this. What's the value add? Seems like it just muddies the waters.

AssertDocumentStoreErrors I like the intent, and it adds value. It can be useful when you aren't sure what you should be asserting client-side, but just want to make sure a static index didn't blow up on the server. A bit of cleanup is needed in the messages. Keep the humor in the comments, don't let them trickle up through to the user code. And see if you can provide a more detailed message in the exception. Perhaps include the errors themselves.

WaitForStaleIndexesToComplete YES - absolutely needed. I do this all the time. But please include a timeout. Waiting indefinitly is probably not good. And I think 50ms is a bit aggressive. How about 100ms instead. And make it public.

CreateSeedData No, sorry. not liking this at all. Let the user worry about putting their data in. They may want to use the new bulk insert api. we shouldn't be wrapping it up like this. If you need to do this, keep it in your own codebase.

NoStaleQueriesListener YES - approved. As is. Thanks. :)

RavenDbTestBase No - It will just confuse people on whether to use this one or the one from Raven.Tests.Helpers. If you want to add extend the RavenTestBase with some extension methods or inherit from it and do some additional things, then fine. But again, dont' focus on seeding data - the user writing the test will want to do that themselves.

Did I miss anything? Sorry so critical. :) I do appreciate your efforts!