3
\$\begingroup\$

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);
        }
    }
\$\endgroup\$
  • \$\begingroup\$ You are not doing unit testing here. Do you want to do unit test or do you want to test that AsaMembershipProvider implements MembershipProvider correctly? In any case you are asserting too much for one test. \$\endgroup\$ – abuzittin gillifirca May 21 '13 at 11:48
  • \$\begingroup\$ The objective is to make sure AsaMembershipProvider implements MembershipProvider correctly. \$\endgroup\$ – user216672 May 21 '13 at 13:15
  • \$\begingroup\$ Check Implementing a Membership Provider. Basic idea is for each member in the "Required MembershipProvider Members" table you should write several tests, covering each testable specification from the description column. You can look at here for an idea of how should test cases look. But it has a restrictive license, so do not use any code directly. \$\endgroup\$ – abuzittin gillifirca May 21 '13 at 13:39
  • \$\begingroup\$ Another word of caution with the linked example, they use DbUnit for populating the data source for each test. I strongly suggest you clean up your data source, such that it is empty for the start of each test and any change to the data source is visible to the reader of the test without reading an external xml file hundreds of lines long. \$\endgroup\$ – abuzittin gillifirca May 21 '13 at 14:54
2
\$\begingroup\$

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)));
        }
 }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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