Monday, December 12, 2011

Scrutinizing another project code base: what to look for

So when you're evaluating an unfamiliar project's codebase, make sure to be careful and not be deceived by how it first looks. On the current project we're working on the code looked like it had a good architecture to begin with, but after working with it for a few weeks you begin to notice some things that are just weird.

Here's some things to watch out for (keep in mind we have an ASP .Net MVC 2 project):
  • Controller logic mixed into the view layer (this is a terrible sign). Not very common in this project but I've seen plenty of it.
  • (Off that bullet) Too much C# code in the view layer that's not really controller logic, but stuff like loops, if blocks, etc. that render different HTML elements are just ugly. 
  • Embedded JavaScript code inside aspx files: In my opinion this is sloppy and pollutes the markup with JavaScript code. What's even worse is JavaScript code that also contains C# code in it!
  • Large JavaScript files. JavaScript code should be treated like your other code -- this includes having some kind of separation to give a modular structure (makes it unit testable too) 
  • Code duplication. This is a big one: why is the code being duplicated? A severe case in this project is two JavaScript files containing the same code but in different places. They even had the same typos! A really annoying case of this is when you have three classes with the same name but in different namespaces.
  • Doing too much in the data layer. Having too much stuff (such as branching logic etc.) in the data layer makes it hard to unit test the bugs that will pop up from it.
  • Commented out code: This is not a red flag but should still be a warning sign. Why is there code that's been commented out? If it's under source control (and it better be) you already have a copy of it, if not then why isn't there an explanation of why it's been commented out?
  • Swallowing exceptions: This is my favorite because often to the user it just looks like their button doesn't work, when in reality an exception was caught, swallowed, and the action just didn't proceed.
So, I've seen all of these things in the project we're on now and this is based on that experience. Your results may vary.

Thursday, December 8, 2011

Selenium tests that fail "sometimes" and Jenkins

We're running Jenkins as our CI server, and part of the automated deployment is to make sure the UI tests pass. Well, sometimes the Selenium tests fail for one reason or another after passing for say, 20 builds. Initially I hypothized the following:
  • Click events don't register
  • IE has a bad day
  • Some kind of timing thing
All of these things have occurred in the past -- especially with IE. So with the help of stackoverflow I went ahead and implemented some things to fix this problem.

The first thing was to Re-run the UI tests that failed a second time. Of course this comes with all the drawbacks and this points to problems with the test itself. In this case that was ok because the test was still useful for confirming a bug was fixed, but we still need the build to pass. But I'll come back to this issue later on in the post.

Well it turns out that while JUnit has the @Rule annotation (see this post) which allows you to intercept test calls, NUnit doesn't have that. Oops. I tried using the NUnit extensibility API but that didn't turn out to well (my fault I'm sure).

So I resorted to this method in our UI test base class, SeleniumTest :

public void RetryFailure(Action testMethod)
{
    if (_retrying)
    {
        _retrying = false; 
        return;
    }
    try
    {
       _retrying = true; 
       testMethod();
    }
    catch (Exception e)
    {
        Console.WriteLine("::::::retrying:::::::::::");
        Console.WriteLine("error was {0}", e.Message); 
        WebDriver.Quit();
        SwitchBrowser(_currentBrowser); 
        testMethod(); 
    }
    _retrying = false; 
}

I'm not too excited about the code quality of this method, but it does work. All it does is attempt to run the test, catch any failures, and then rerun the test again. Here's what the beginning of a test that uses this looks like:

[TestCase] 
public void AdminCancelANeedViaOfficeCalendar()
{
    if (!Retrying)
    {
        RetryFailure(AdminCancelANeedViaOfficeCalendar);
        return;
    }

// ... more code

As you can see, we have to have this additional bit of logic at the beginning of each test. Now, it would have been easy to just encapsulate the test code in some kind of loop and then repeat it that way, but this allows the SeleniumTest.RetryFailure method to have all of the control.

Alright, so the next step was to generate screenshots of test failures. This is pretty easy as Selenium already provides a way to do this by casting the WebDriver to ITakesScreenshot. So nothing exciting there.

Now to integrate this into Jenkins. At this point, we have a bunch of failure screenshots being archived as artifacts, but you have to actually go look for them to view them. I'm lazy so instead I took a shot at providing a custom HTML report of the failures with their screenshots:
Summary of test failures

So the screenshot is from a page that is accessible via the Jenkins project page. This means that we can be lazy and still get the information we need.


 
UI Test Failures link shows up on the project page
 The HTML Publisher plugin takes care of the details, all we have to is generate the actual page. This is done once again inside SeleniumTest.

 private void BuildReport(string reportFile, string testName, string testImagePath)
{
    if (!File.Exists(reportFile))
    {
        using (StreamWriter sw = new StreamWriter(new   FileStream(reportFile, FileMode.Create, FileAccess.Write)))
        {
            sw.WriteLine("<html><body>");
             sw.WriteLine("<h1>Test Failure Screenshots</h1>");
         }
    }
    try
    {
          using (StreamWriter sw = new StreamWriter(new FileStream(reportFile, FileMode.Append,  FileAccess.Write)))
         {
             sw.WriteLine("<div>");
             sw.WriteLine("<h2>" + testName + "</h2>");
             sw.WriteLine("<img src=\"" + testImagePath + "\"/>");
             sw.WriteLine("</div>");
         }
    }
    catch (Exception e)
    {
         Console.WriteLine("Problem writing report: {0}", e.Message); 
    }
}

Pretty simple, probably could be moved to somewhere more fitting. In the mean time though, it works well enough.

Anyway, the point of all of this was to just allow us to see why tests fail but also retry them to see if they fail reproducibly. It turns out that a couple of the tests repeatedly fail the first time but not the second, so there's probably some work to do.

See you next time.

Sunday, December 4, 2011

Another abstraction layer on top of PageObjects with Selenium

Hi everyone,

As I mentioned before, on this project our Selenium 2 tests are extremely important. We've tried to keep the test code on the level of the production code, so it naturally has its own object model. We already use PageObjects extensively in the project.

Recently a few bugs popped up that we reproduced with Selenium tests that were pretty similar and required similar actions. Here's an example of some duplicated code:

// first step: log in as a temp to accept a need
LogOn login = new LogOn(WebDriver);
login.Go();
login.Login("Temp", "temporary");

TempNeeds need = new TempNeeds(WebDriver);
need.ViewFirst();
need.Accept();

login.Logout();
login.Go();
login.Login("Chris", "foo");

Billing billing = new Billing(WebDriver);
billing.Go();
billing.CreateNewInvoices();
billing.UpdateAndReview();
billing.FinishCreate();

Turns out this bit of logic is common in our tests, so it gets duplicated everywhere. So in the interest if minimizing the amount of code, I modified it to look more like this:

Temp temp = new Temp(WebDriver, "Temp", "temporary");
temp.AcceptFirstAvailableNeed();

Admin admin = new Admin(WebDriver, "Chris", "foo");
admin.CreateInvoices();

This code does the same thing, it's just been hidden by another abstract layer that represents the different users of the site and what they would do. Usually, each method off of a user would interact with many different PageObjects. The idea is to make the code reveal its purpose more by telling you what the user is trying to do, not the different PageObjects that need to be used.

Anyway, we're trying this approach out now for newer tests so we'll see how it goes. Hopefully the more tests we write and the more objects we build up, the less new Selenium code we'll have to write.

Wednesday, November 30, 2011

Use screencasts for your bug reports

Note: So finals are about to start, you might not see me for awhile.

Anyway,

On the project I'm on now, we're working directly with the client (the owner of a local business) on a web application. We took over the project from another company working on it and started with some bug fixes. In my experience the most difficult part of tracking and killing bugs reported by someone else is simply reproducing them.

Let's take a look at a bug report style you might have encountered (I certainly have):
Bug: Clicking on the SaveLotsOfMoney button causes the browser to explode

Login as Jim
Click "SaveLotsOfMoney" button
"Server error" is displayed right before the browser explodes and crashes

Attached: a screenshot of the browser exploding. 

Alright, this is actually a good one because at least they stuck a screenshot on it and they were nice enough to enumerate the steps they had taken (as opposed to "it just crashed").

Now you've just taken the project and you have no idea how the code works. So you try and reproduce it, but the SaveLotsOfMoney button works fine for you. Oops?

Turns out the actual problem was that during previous testing the reporter did something unrelated, data was changed, and the guy before you forgot to do a null check and now the browser has exploded. It doesn't happen for you because you're making your tests reproducible (of course).

How do you prevent this? Make them do it again? Now they can't reproduce it! They didn't know that what they did previously affected that test, because it's not their job to know the details, so of course they don't include it in their bug report.

The solution to this that our current client uses is to make screencasts of the bug in action. This is very important, because it means you can see everything that they were doing that caused the bug. It can vary, and in my case it was a detailed error message (a "foreign key violation -- big help") that was shown. In another case I was able to see signs of data they had previously changed, which led to reproducing it.

So: don't use those old crusty bug reporting methods, try making screencasts of them instead. The client mentioned uses a tool called Jing (disclaimer: I don't work for them) to record their screencasts and it looks good to me, though I have no personal experience with it.

Monday, November 21, 2011

Get up to speed on a new project by writing Selenium tests

Alright. So I've taken the task of maintaining and extending an ASP .Net MVC 2 project with a friend of mine. It's not too big, but there are some things working against us:
  • Neither of us have done ASP .Net development before
  • We had to set up a new database (this was actually the hardest part) 
  • There are no tests for the project
  • The javascript looks like our javascript back in the beginning of the Recordings project ... we've learned better since then.
Actually, let me emphasize that last point. I'm a big fan of automated testing, so much that I feel hesitant to modify code that isn't under test (excluding the simple stuff, of course). Especially on an unfamiliar code base. That's why one of the things we've been doing and really emphasizing on this project is automated UI tests with Selenium 2 WebDriver.

Writing tests using Selenium has helped us learn the functionality of the application as well as establish a good suite of tests for future modifications. And at the moment they are more useful than unit testing due to the difficulty of unit testing parts of the code.

In addition, we're making sure to keep our tests nice and clean using PageObjects and also making them reproducible when they mess with the database. Eventually we'll have these tests picked up by our CI server when it builds the project before deployment.

Of course, that doesn't mean we're going to go without unit testing the code. There are a couple static classes from the ASP .Net libraries that are mixed in with areas we'd like to test -- but using some questionable hacks we've been able to work those out. And after burying said hacks in another layer of abstraction, we can write nice clean unit tests like we all enjoy.

So: Now that we're on a new project things should pick up with this blog again. Recordings is live and complete, so there hasn't been much programming for a few weeks.

See you next time, though.


    Monday, November 7, 2011

    Log4net in a not-.NET application

    Hi everyone,

    So I've moved on to a couple of other projects (Recordings is now live and complete enough). One such project is an old legacy VB6 application that I'm extending with C# via COM interop.

    The situation I have is this:
    • I would like to use Log4net in the C# code, compiled as DLLs and imported into the VB6  project
    • I would like to configure Log4net via an xml config file, but it won't have to change at runtime. 
    So, here's how that's accomplished:

    The log4net.config file is added as an embedded resource in the C# class library project. It looks like this:

    <?xml version="1.0" encoding="utf-8" ?>
    
    <log4net>
      <appender name="DebugFileAppender" type="log4net.Appender.RollingFileAppender">
        <file value="J:\Chris\Source\citysiege\main\logs\" />
        <appendToFile value="true" />
        <rollingStyle value="Date" />
        <maxSizeRollBackups value="30" />
        <datePattern value="yyyy-MM-dd'.txt'" />
        <staticLogFileName value="false" />
        <layout type="log4net.Layout.PatternLayout">
          <conversionPattern value="%date [%thread] %-5level %logger - %message%newline"/>
        </layout>
        <lockingModel type="log4net.Appender.FileAppender+MinimalLock" />
      </appender>
    
      <root>
        <level value="DEBUG" />
        <appender-ref ref="DebugFileAppender" />
      </root>
    </log4net>
    

    A static class method serves as a wrapper around the log4net library and an entry point to configure it:

                var assembly = Assembly.GetExecutingAssembly();
                var stream = assembly.GetManifestResourceStream("CitySiegeLogic.log4net.config");
                log4net.Config.XmlConfigurator.Configure(stream);

    Simple right? Well, took a little while to figure out. Also, if the config file is in a subfolder be sure to add that to the path along with the namespace, so for example "CitySiegeLogic.logging.log4net" if its under /logging.

    Thursday, September 22, 2011

    Workflow on the 2-person Recordings project

    Hi everyone. Today I'm going to share the workflow we use on our project. Mostly for reference, I'll come back in a year and see how things change. Hopefully we'll be on other projects by then.

    So let's just see the process for fixing a bug and deploying the fix to production. That will go through the entire process.

    Step 1: Observe the bug happening

    The bug I'm talking about is the canned graphs problem. Certain types of graphs showed this result:

    The Undesirable Result

    Great. After confirming that it's a Real Bug and not just user error, I create a new ticket on our bug tracker, Unfuddle: 

    The bug report: You can see it's "fixed," that's because this is a current screenshot. 


     Step 2: Reproduce and isolate the bug with tests

    In general I try to trap every bug with a unit or integration test. This particular bug had a good 'integration' test case for it, but I also wrote a UI test with Selenium

    You can see that I'm exciting about Selenium because I used the phrase "real live selenium ui test."

    Step 3: Bug fix

    The build jobs that run during merges
    The actual bug fix in this case was trivial (see the post mentioned above). After making the fix, the changes are merged from my dev branch into the Test branch (our main 'stable' branch). This merge is picked up by the build server, which builds the project, runs the tests, packages it into a WAR file, and archives it.

    Step 4: Deploying to production 

    Jobs running for Production
    Once the build is good, the next step would be more thorough manual testing to determine any tweaks, additions, etc. before we stop working on the feature. In this case though, we can just merge to our Production branch. 

    Now the fix is ready to deploy, so we can run the "Production Deploy" job. This will upload our WAR file to the live server which tomcat will automatically redeploy. After running "Production Status" to confirm the website is still running, we can visit it to confirm it's working.


    Well, that's pretty much it! The process is more exciting for new feature additions, but it's basically the same process. Check back in a year to see how the process changes....