ASP MVC Controller Unit Test So I've been writing a unit test using Rhino Mocks to a controller in my ASP MVC application. The test passes, but I'm interested in your opinion and if there's something I should be doing differently or what it could be.

Here's my test method:

    [TestMethod]
    public void TestSlideView()
    {
        // Arrange
        var repositoryMock = MockRepository.GenerateMock&lt;ISlideRepository&gt;();
        var controller = new SlideController(repositoryMock);
        var expectedSlides = new List&lt;SlideModel&gt;();
        expectedSlides.Add(new SlideModel
        {
            Id = "id"
        });
        repositoryMock.Stub(x =&gt; x.GetSlides("id")).Return(expectedSlides);

        // Act
        var actualView = controller.Slide("id") as ViewResult;
        var actualData = actualView.Model;

        // Assert
        Assert.IsNotNull(actualView);
        Assert.IsNotNull(actualData);
        Assert.AreEqual("Slide", actualView.ViewName);
    }


Here's the controller under test:

    public ActionResult Slide(string slideid)
    {
        var slides = slideRepository.GetSlides(slideid);

        return View("Slide", slides);
    }


The repository the controller uses is here:

public interface ISlideRepository
{
    List&lt;SlideModel&gt; GetSlides(string id);
}

public class SlideRepository : ISlideRepository
{
    public SlideModel Slides { get; set; }
    public List&lt;SlideModel&gt; SlideList { get; set; }


    public List&lt;SlideModel&gt; GetSlides(string id)
    {
        string cs = dbPath;
        string slideid = id;

        using (SQLiteConnection con = new SQLiteConnection(cs))
        {
            var listOfSlides = new List&lt;SlideModel&gt;();
            string stm = "SELECT * FROM Slide WHERE ID = " + slideid;
            con.Open();

            using (SQLiteCommand cmd = new SQLiteCommand(stm, con))
            {
                using (SQLiteDataReader rdr = cmd.ExecuteReader())
                {
                    while (rdr.Read())
                    {
                        listOfSlides.Add(new SlideModel
                        {
                            Id = rdr["ID"].ToString()
                        });
                    }

                    rdr.Close();
                    SlideList = listOfSlides;
                }
            }

            con.Close();
        }

        return SlideList;
    }
}


Any input is greatly appreciated!
The string "id" is used three times in TestSlideView(): make it a const string instead. 

Same for "Slide", as used in TestSlideView() and Slide(string slideid).



Don't name something SlideList, name it Slides. And yes, I know you named it SlideList because you already have a property called Slides, but Slides isn't an appropriate name for a single SlideModel anyway.

Even worse: listOfSlides.



Your variable names don't tell me what they contain: cs, stm, etc. I can figure out that "cs" means "connectionString", but I shouldn't be wasting effort on such things.

Also, why bother assigning dbPath to cs? Why not simply use dbPath (and perhaps rename that to connectionString)? 

Same for assigning id to slideid -- which should be named slideId since it is a compound word. Matter of fact, why isn't the parameter named slideId in the first place?



"Danger, Will Robinson!" You code is now vulnerable to SQL injection:

"SELECT * FROM Slide WHERE ID = " + slideid;


Use the Parameters collection instead.

Also: looping through a SQLiteDataReader seems quaint to me. Why not store the result in a DataTable and work with that? Or even better: why not work with an ORM like NHibernate or Entity Framework?