current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I'm new to using NUnit and have written a test to check for the existence of a database table. I have the below code that should check whether a new table named NewTable has been created in the database. It works correctly but I can't help feeling there's probably a better way of doing this. Thanks.

using(var conn = context.NewConnection()) {
    var tables = conn.GetSchema("Tables");
    foreach(System.Data.DataRow row in tables.Rows) {
        foreach(System.Data.DataColumn col in result.Columns) {
            if(row[col.ColumnName].ToString() == "NewTable") 
                Assert.Pass();
        }
    }
}

Assert.Fail("NewTable not created");
share|improve this question

1 Answer 1

up vote 4 down vote accepted

I would do something like this instead -

using(var conn = context.NewConnection()) {
    var table = conn.GetSchema("Tables");
    var tableNames = table.Rows.Cast<DataRow>()
                                            .Select(x => x["TABLE_NAME"].ToString())
                                            .ToArray();
    Assert.That(tableNames.Contains("NewTable"), Is.True);
}

Tests are usually structured in Setup-Act-Assert fashion. A good unit test is one that tests just one condition on an action.

Multiple asserts are practically unavoidable in certain cases, but having logic to decide when Assert should pass or fail isn't a good practice. The code under test should have the logic to branch, the test should only look at expected v/s actual results in a given scenario.

In this case, our setup is - we get the schema from the database, and get the list of column names.

Our logic (Act) is that certain column name must exist. So we write an assertion to ensure that.

In other cases, there is an action and a result, the assertion is on the result. In this case, it is more of testing a state, so there isn't an assertion on the result of an action.

share|improve this answer
    
Thank you for your answer! I think a simple explanation of why this is better ("only one assert", "better use of existing constructs", ...) could help. –  Quentin Pradet Feb 27 '13 at 7:23
    
wouldn't that check that there was a column returned by the GetSchema method named "NewTable"? I'm not sure that works. I want to check that a there is a table named "NewTable". I've just plugged that code in and the test now fails I'm afraid –  acqu13sce Feb 27 '13 at 7:26
    
@QuentinPradet - added a few lines on my thoughts around testing, hope it helps. –  Srikanth Venugopalan Feb 27 '13 at 7:31
    
Your answer inspired me to a better solution than I had. I have submitted an edit to the code so that it solves the exact question asked. Thanks –  acqu13sce Feb 27 '13 at 7:34
    
@acqu13sce - I think you are right, the code might not work as it is. I didn't bother executing the getschema against a database. I'll try to make an edit that reflects something more real. Btw - I have included your edit. –  Srikanth Venugopalan Feb 27 '13 at 7:37

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.