Wednesday, March 19, 2014

Subject: Further education

Alternate title: Exercises in cowboy style software development

I stole this art from: http://blog.siteground.com/siteground-staging/

Sometimes we're not happy with the way things are going on around us. Lately, I've been trying to make a difference in many different things. I'm coming up with a plan of action but decided to set off on the path before the plan is finalized. I think of it as a RAD (Rapid Application Development) form of self improvement. I have a reasonably well defined goal of where I want to go and know the steps I'm taking will get me closer. So I'm off...

I think that a lot of cowboy development happens where I work. That type of behavior skips a lot of what I consider to be good practices in software development. So, I'm trying to get the other members on the team to see the value of certain practices. In this case it's writing effective unit tests. Because of the way my mind works I approach things by seeing what's wrong with it. That helps to explain the structure of the message I sent.

* It's worth noting that the developer that created this unit test is no longer with the team. Though an exceptional developer in most every other way I think he fell a bit short here. Even if he was here he wouldn't be embarrassed by this being used as an example. He was, and I believe still is, always open for an intellectual discussion.

 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Subject: Further education

Below is a unit test from the OurGroup.OBP.EnrollmentPortal project. AKA: OurBigProject

Please review this unit test and indicate its shortcomings. I found at least two direct shortcomings and more than two indirect ones.

Note: This is the only unit test in the solution for ASourceii Labs.

    [TestMethod]
    public void ASourceii_GetLabs()
    {
      IOBPDataSource source = (IOPBDataSource)context.Lookup(@"DataSources\OurGroup.DataSource");
      List<LabTest> labs = source.GetLabTestsForPatient(TestPatient, DateTime.Parse("10/22/2002"), DateTime.Now, TestUser);
      foreach (LabTest lab in labs)
      {
        if (lab.LabTestName.Contains("CANCER")) throw new Exception("You dumbass");
      }
    }


Responses expected by COB today.
 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

My worst fears were realized. I got nothing back. 

Utilizing some skills I recently picked up in a management focused class I'm taking I responded like this. Basically, that means I didn't admit that no one responded. I praised them for the success they might have had in hopes that some guilt might develop for their lack of participation when they believe others have.

 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Subject: RE: Further education

Not bad for you simple cowboy developers.

Zone of Rant
One of, if not the most important, reasons for unit tests is to allow you to make modifications to code and be well assured that your changes did not break the code. Not only that the code fails to throw exceptions but that it still meets each and every expectation held for it. In the case of OurBigProject, like many of our applications, we can't replace the antiquated and difficult to use data layer without substantial risk. A solid suite of unit tests would significantly reduce that risk. You can pay some now or you can pay a whole bunch more later.

First things first:
- What is this test verifying? There are no comments or association to the requirement or test case specifying what is being verified. It's left up to whoever comes upon it to guess.

Other critical items:
- There are no Asserts. In MSTest the standard pattern is to use an Assert for the validation.
- Near and dear to one of our team members's. heart is the unprofessionalism displayed in this code. I got read the riot act for using the word "poop" as a temp variable in some work I wasn't finished with and this contains the word "dumbass". This code, in this exact form, was sent to the Another Organization. Besides making us look, unprofessional the boss type people wouldn't be happy if they found out about this.
- This is the only test for Labs. It only verifies that none of the returned records contain the word "CANCER"**. If no records were returned then the test passes.


Other issues:
- **The method string.Contains is case sensitive. So the word "Cancer" would not be caught by this test.
- There is no explanation of what the input or output test data is. You don't know, without some digging, what the makeup of any of those is.
- This test throws an exception. In some testing frameworks if an exception is thrown then the system stops and none of the other tests are run. All tests in a suite should be able to be executed regardless of the pass or fail status of another test. IF you want to create a dependency between tests then do it in code using the available methods.
- The thrown exception doesn't tell you enough. (Not for some of use anyway) It should log the reason for the failure.
- There should be other tests. For example: One with a list of the expected results. By simply including a start and end date an exact list could be retrieved and compared to a benchmark.*

- It's hard to see here but this test is dependent on access to an external data source. In this case it's a test instance of a web service (That isn't working BTW) that pulls data out of a production system that we don't have full control of. One of the beauties of dependency injection is that you the developer could isolate the environment so that the external dependency is removed. Especially because it's dependency injection you could have it run both ways. Once against an environment you have total control over, making it a true "unit" test, and once against the full environment.


*Most every object in the OurBigProject project can be serialized. That means the input objects and expected benchmark values can be stored in files. That makes it easy to create and execute a wide variety of test scenarios. (I'll show how this is can be accomplished in my next installment.)

In conclusion:
Unit tests are more than a way for you to exercise your code outside of the application that will use it during development. It insures that the code does what is expected of it. The time saved by not writing an adequate set of unit tests will quickly be lost the next time anyone goes in and makes a modification. Relying on the test process at the end of the development cycle to catch mistakes you have made is failure from the onset.

 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

I don't think I'll be able to realize the effectiveness of this effort any time soon. Maybe if I open up a solution in the future and see more than one unit test per method.

Oh, and I know I need to get better at formatting my posts. I'll check into that.