Unit test provider roles

Question

I am unit testing this role provider. I have a few asserts in the code for my test. I am sure there are more test I could preform. Does anyone have any suggestions for more test for this role of the membership provider?

[TestMethod]
    public void TestDeleteUserAccess()
    {
        try
        {
            string sRoleName = "TestRole";
            string sUsername = "test.user";

            // Use a known user for relationships
            AsaMembershipProvider prov = this.GetMembershipProvider();
            MembershipUser user = prov.GetUser(sUsername, false);

            // Create a new role
            AsaRoleProvider roleProv = this.GetRoleProvider();
            roleProv.CreateRole(sRoleName);

            // Verify that role exists
            bool bRoleExists = roleProv.RoleExists(sRoleName);
            Assert.IsTrue(bRoleExists);

            // Add users to that role
            string[] usernames = new string[] { sUsername };
            string[] roleNames = new string[] { sRoleName };
            roleProv.AddUsersToRoles(usernames, roleNames);

            // Verify that user is in role
            bool bRelationExists = roleProv.IsUserInRole(sUsername, sRoleName);
            Assert.IsTrue(bRelationExists);

            // Check various methods for finding role information
            string[] matchUsernames = roleProv.FindUsersInRole(sRoleName, "userx");// find constant 
            foreach (string matchUsername in matchUsernames)//if match.length = 0 then roles were returned
            {
                Trace.WriteLine("Found in role " + sRoleName + ", user " + matchUsername + ". ");//check something instead of trace
            }

            string[] matchRoleNames = roleProv.GetRolesForUser(sUsername);
            foreach (string matchRoleName in matchRoleNames)
            {
                Trace.WriteLine("Found for user " + sUsername + ", role " + matchRoleName + ". ");
            }

            // Remove user from the role
            roleProv.RemoveUsersFromRoles(usernames, roleNames);

            // Verify that user is no longer in the role
            bRelationExists = roleProv.IsUserInRole(sUsername, sRoleName);
            Assert.IsFalse(bRelationExists);

            // Delete the role
            roleProv.DeleteRole(sRoleName, true);

            // Verify that no longer exists
            bRoleExists = roleProv.RoleExists(sRoleName);
            Assert.IsFalse(bRoleExists);
        }
        catch (Exception ex)
        {
            LogMessage(ex);
            Assert.Fail(ex.Message);
        }
    }

Answer 1

I am going to suggest a few improvements.

Most of the principles for code quality apply just as well to test code as it does to production code: Your tests should follow Single Responsibility Principle. A test should ascertain one feature; such that any change or discovery or addition of a feature does not cause you to fix many unrelated tests. Ideally any bug introduced will cause one test to fail and that test failure will direct the programmer to the cause of the bug.

You should use Intention Revealing Names. The names of your tests should tell the reader of the tests what is being tested.

For example Implementing a Role Provider says: "CreateRole method; Takes as input the name of a role and adds the specified role to the data source for the configured ApplicationName." One test case for that spec could be:

 public void WhenICreateARoleItExists() {
        roleProv.CreateRole(A_VALID_ROLE_NAME);

        Assert.IsTrue(roleProv.RoleExists(A_VALID_ROLE_NAME));
 }

This code is derived from your first assertion. Notice removal of code related to other assertions made the test more readable.

You can initialize your System Under Test in a [SetUp] method and assign it to a field, here roleProv, so that common initialization code does not clutter the reader's view. You should also extract repeated constant values, here A_VALID_ROLE_NAME, to const members, and name them as descriptive as you can.

You should also test that what your system should not do. For example, the document says : "You should throw a ProviderException if the specified role name already exists for the configured ApplicationName."

  public void MultipleRolesCannotBeCreatedWithSameName() {
        var sameName = A_VALID_ROLE_NAME;
        roleProv.CreateRole(sameName);

        Assert.Throws<ProviderException>(delegate {roleProv.CreateRole(sameName);});
  }

Any unexpected exception should not be caught and indicates a broken test. Your test runner should , and most probably does, log its message and stack trace. Any expected exception should not cause the test to fail. You can remove your try/catch above.

I noticed that you write the values to the Trace if a collection is returned from a tested method. That is totally useless. If a test does not Assert, it is not a test. A test should fail when the functionality it is testing is not working.

 // Names are just to give you an idea
 public void WhenIAddSomeUsersToSomeRolesTheirRolesIncludeAllOfThoseRoles() {
        // Given some users
        var userNames = new string[] { A_VALID_USER_NAME,  ANOTHER_VALID_USER_NAME};
        foreach(var userName in userNames) {
             createUserWithName(userName);
        }

        // And some roles
        var roleNames = new string[] { A_VALID_ROLE_NAME,  ANOTHER_VALID_ROLE_NAME};
        foreach(var roleName in roleNames) {
             createRoleWithName(roleName);
        }

        // When I add those users to those roles
        roleProv.AddUsersToRoles(userNames, roleNames);

        // Each users roles contain all of those roles
        // This could be done more idiomatically by someone more familiar with C#
        foreach(var userName in userNames) {
             Assert.That(roleNames, Is.SubsetOf(roleProv.GetRolesForUser(sUsername)));
        }
 }