Viewbagdatasupport

[segilbert]

Added support for fluent validation for ViewBag and ViewData in the ViewResultTest class using WithViewBag and WithViewData methods respectively. Added ImpromptuInterface reference to check ViewBag dynamic collection for member name passed in.

http://nuget.org/packages/ImpromptuInterface/

Added unit tests in ViewResultTestTests class to validate 2 new methods.

Use: Adding unit test for ASP.NET MVC default HomeController

    [Fact]
    public void Index_RenderView_WithViewBagMessage()
    {
        // Arrange
        HomeController controller = new HomeController();

        // Act
        // Assert
        controller.WithCallTo(c => c.Index())
                  .ShouldRenderDefaultView()
                  .WithViewBag<string>("Message")
                  .Should()
                  .Be("Modify this template to jump-start your ASP.NET MVC application.");

    }

[robdmoore]

Hi @segilbert! Thanks for the PR :) Have you been using this library for long? How have you been finding it?

I think it's a good addition to have view bag and view data checking. I have a few questions below that come to mind after reviewing the code.

I'm happy to devise / code up some solutions to these questions if you don't have any more time, but given you've already done some of the work I figure I'll put my thoughts here so we can work on it together if you like?

• I'm curious about the spacers between namespaces; I'll be honest - I've never seen that technique used before?
• Do you think there should be an assertion that the type given is the same as (or assignable to) the type of the object stored in that key of the viewbag / view data? I'm not sure what would happen at the moment, possibly a type cast exception, which wouldn't be as descriptive as some sort of assertion exception.
• To keep consistency with the rest of the library I think we should make it so multiple viewbag and view data checks can be made, and possibly the model error checks as well? In that case the WithViewBag / WithViewData methods wouldn't return the value, but rather a new type of object that gave some sort of assertion methods (possibly similar ones to the model - one that takes a predicate, one that takes an action that can be used to construct actual assertions and one that takes an object and compares using .Equals?) or alternatively the other methods if you just want to check there was something in that key with that type?
• Should we allow you to check if there is a viewbag / view data entry for a given key without worrying about the type? If we did that should we allow further assertions on the object in that key or not?

Some of those questions might be a bit hard to understand without a code sample so let me know if you want me to clarify any of them.

Thanks again!

[robdmoore]

Oh and a couple of other things:

• Looks like there are a number of different ways of getting the view bag properties via a string including Impromptu: http://stackoverflow.com/questions/4939508/get-value-of-c-sharp-dynamic-property-via-string. Given this is in a test I don't really care about the performance overhead of reflection. Do you think it's worth pulling in a dependency just for that one line in the library?
• I'm also wondering if it would be worth adding an overload for the ViewBag one that takes an Expression<Func<dynamic, object>> so you can also do .WithViewBag<string>(v => v.Message) as well as .WithViewBag<string>("Message")
• I don't want it to be too wordy, but I'm wondering if .WithViewBag should be something else to make it read grammatically. .WithViewData reads OK, but .WithViewBag doesn't indicate it's talking about a specific property, but seems like it's talking about the bag in it's entirety. In saying that I don't have any great suggestions instead of it. Hmm, maybe .WithViewBagProperty?
• The implementation for the view bag check is looking in the view bag in the view result, I can't remember if that is automatically copied from the controller view bag or not? When setting stuff in view bag I'll normally do it on the controller view bag object.

[segilbert]

Rob,

Thanks for the consideration and thoughtful analysis. I tossed this together quickly to help me setup a more fluent and expressive xUnit Test Framework template for VS with the intention of expanding. Basically creating an xUnit Test Framework Template for ASP.NET MVC projects in VS; includes xUnit, NSubstitute, TestStack.FluentMvcTesting and MVCRouteTester. As soon as I started using TestStack.FluentMvcTesting I loved it. I'm also looking at MvcContrib and MVCRouteTester to test routes ... leaning towards MVCRouteTester at the moment. Have you used either for testing routes?

I'm open to helping, let me try to address your list of questions.

• Spacers between namespace is a standard my internal work teams have adopted to allow classes with many diff references standout quickly to the human eye and appears more organized. This is more preference than anything. You may not agree or find it helpful, ultimately this is your baby, I understand you make the call. No skin off my back either way.
• This makes sense to me, agree, more descriptive we can be with error messages the better.
• Yes, agree. Makes sense to remain consistent, plus this makes these additions more usable.
• If we provide ability to check without specifying type it would make sense to do additional assertions, especially the type. Is your goal here to provide the assertion extensions or something else?
• Regarding Impromptu, suppose to be faster than reflection plus the example you reference does not work, at least not for me. I received null object, could never determine the type to get the property info. It was my understanding this is because the underlining structure is a dynamic view data dictionary. I could have been doing this wrong. Impromptu of course just works. My initial preference was to address without an external dependency but was unable to.
• Good idea. Like adding the support for lambdas.
• I'm a fan of clarity, conventions and being explicit ... that said, .WithViewBagProperty sounds fine to me.
• Yes, normally set on ViewBag in Controller which sets ControllerBase.ViewBag. This is my mistake on the tests. In debugging my use of the new features in a new ASP.NET MVC app testing the default HomeController ViewBag messages, the ControllerBase.ViewBag messages set on the controller are copied down to the ViewResult.ViewBag structure ( which behind the scenes is using a ViewData structure ). The tests should be tweaked to set the ViewBag on the controller not ViewResult.

[robdmoore]

Tonight was spent figuring out how to get tsqlt tests running in TeamCity :P I'll get back to you tomorrow evening! FYI I'm GMT+8 :)

Is this the route library you mean? http://mvcrouteunittester.codeplex.com/ I've never used that before, but the syntax doesn't look anywhere near as nice as MvcContrib? I personally use a modified version of MvcContrib that uses NSubstitute instead of Rhino.Mocks and also checks that the route data returned from the route check, when passed into Url.Action will generate the same route that you are testing against. I've been meaning to open source it, but I haven't found the time. If you are interested in seeing it let me know.

[robdmoore]

• To keep consistency with the other TestStack projects I'd probably be inclined to keep the namespaces without the spacers. I can see why you might want to do it though so it's really interesting to me (to see different approaches). My view on namespaces is I largely ignore them and just let ReSharper take care of them :P
• Cool. We should add an explicit check for the type then - there are existing examples for how I did that for the model errors
• So I'm thinking something like .WithViewBagProperty<string>("Key"), .WithViewBagProperty("Key", "hello"), .WithViewBagProperty<string>("Key", s => s.StartsWith("he")) and .WithViewBagProperty<string>("Key", s => { Assert.That(s, Is.StringEndingWith("lo")); ...})
• I think the idea would be to allow for .WithViewBagProperty("Key"). In terms of multiple assertions I'm thinking this should be possible: .WithViewBagProperty("Key", "hello").AndViewBagProperty("Key2").AndModel(model).AndModelError("Errorkey") that would mean we had to check the view bag before the model unless we allowed the viewbag calls to be chained off the model ones too. What do you think?
• I agree Impromptu will be faster, but for these kind of unit tests the difference will be milliseconds and thus that kind of optimisation doesn't matter. I'm happy to take a look to see if we canget it working with just reflection. If we can't then we can always IL-merge Impromptu in so it's not a dependency of the library, but rather an unobtrusive internal concern.
• So lets add overloads for all the above suggestions in the third bullet point so that we can replace key with v => v.Key
• For consistency let's go with .WithViewDataProperty as well then?
• It's OK what you did is right on reflection, I'm pretty sure hte View method on the controller ensures the viewbag is copied to the view result so it's all good :)

[robdmoore]

If you are happy to run with some of this feel free. It will probably take me a couple of weeks to get to this because I'm tied up in a couple of other projects at the moment.

Also, I've been chatting with some other TestStackers and we were thinking that it might actually make sense to include route testing in this library. I would take the code I have that uses NSubstitute and put it in there and possibly tweak the fluent syntax to make it better. What do you think?

[segilbert]

I'd be happy to run with this, see what I can tackle this week. Will touch base early next week.

[robdmoore]

Awesome! Looking forward to it mate

[SeriousM]

I'd be happy to run with this, see what I can tackle this week. Will touch base early next week.

any updates on that available?

[segilbert]

I've been treading water lately. If you are still interesting in adding
this functionality I likely will be able revisit later in the month. Let
me know.

On Fri, May 2, 2014 at 11:31 AM, Bernhard Millauer <[email protected]

wrote:

I'd be happy to run with this, see what I can tackle this week. Will touch
base early next week.

any updates on that available?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-42044282
.

Sean T. Gilbert
http://www.i-m-code.com/blog/

[SeriousM]

Well, it's not that necessary but it would have been nice if it's done already. I suggest to close this issue.

Cheers

[brunobertechini]

Have installed latest nugget package and no WithViewBag support...

Do I need anything else ?

Bruno

[robdmoore]

Hi @brunobertechini,

This work hasn't been merged into the codebase because there is still work to do on it.

Feel free to look at the comments above and take the work so far and get it into a PR of your own :)