Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Added consistent model property to view result variants #4901

Merged
merged 3 commits into from
Jun 28, 2016
Merged

Conversation

cjqian
Copy link
Contributor

@cjqian cjqian commented Jun 23, 2016

ViewResult, PartialViewResult, ViewComponentResult.

This resolves #4813.

@dnfclas
Copy link

dnfclas commented Jun 23, 2016

Hi @cjqian, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

Assert.Same(model, viewResult.ViewData.Model);
Assert.Same(controller.ViewData, viewResult.ViewData);
Assert.Same(controller.TempData, viewResult.TempData);

if (model != null)
{
Assert.IsType(model.GetType(), viewResult.Model);
Assert.NotNull(viewResult.Model);

Assert.IsType(model.GetType(), viewResult.ViewData.Model);
Assert.NotNull(viewResult.ViewData.Model);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure what the purpose of this block is.. this is already covered by L38 in the existing code, and L37-38 - suggest deleting this

@rynowak
Copy link
Member

rynowak commented Jun 24, 2016

I'm not sure what happened with your gits and commits, but there's plenty of them showing up in the history for this PR. Have someone look over your shoulder when this is ready to merge ⌚

@cjqian
Copy link
Contributor Author

cjqian commented Jun 24, 2016

🆙 📅

@@ -34,6 +34,7 @@ public void ControllerView_InvokedInUnitTests(object model, string viewName)
var viewResult = Assert.IsType<ViewResult>(result);
Assert.Equal(viewName, viewResult.ViewName);
Assert.NotNull(viewResult.ViewData);
Assert.Same(model, viewResult.Model);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove the useless block at lines 42-46. Same for the other test you're changing. This is going to come up every time someone looks at these tests.

@cjqian
Copy link
Contributor Author

cjqian commented Jun 28, 2016

🆙 📅

(Removed redundant lines, created issue #4906.)

@rynowak
Copy link
Member

rynowak commented Jun 28, 2016

So #4901 (comment) ???

@rynowak
Copy link
Member

rynowak commented Jun 28, 2016

:shipit:

@cjqian cjqian merged commit e51a118 into dev Jun 28, 2016
@cjqian cjqian deleted the cjqian/4813fix branch June 28, 2016 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants