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....

Sunday, September 18, 2011

More bug-fixing with Selenium WebDriver and PageObjects

A problem, I'm definitely connected to the internet.
Hi everyone. I spent the past few days tracking and trying to reproduce another problem we've been having on the server. The problem is with the "recommend box" -- basically a way you can vote on your favorite songs or concerts. The problem happened seemingly randomly when you tried to recommend a song or a concert:

Oops. This bug had a few annoying characteristics:
  • It was hard to reproduce and had to obvious order to causing the problem
  • I couldn't reproduce it locally 
  • The server stack trace was useless -- because we use an open session in view filter the root cause was a database problem but only showed up at the end of the filter when the transaction is committed 
Not a good combination. After spending a couple days just experimenting and try to reproduce it, I finally sat down and decided to squash it for real. It was clear that without being able to reproduce it consistently, it was going to be a hard fix. It was also clear that it was a database related problem, so the first steps were:
  1. Restore the database with a set of test data
  2. Try to reproduce the problem locally 
  3. Record each action taken until the problem occurs
  4. Repeat until its consistent
Luckily, that actually didn't take too long. I was able to isolate it to about 4 or 5 manual steps. The next part was to automate this process. Since it had no obvious starting point in the code, it was a good candidate for a UI test with Selenium WebDriver.

As with the previous post, we make heavy use of PageObjects to abstract the details of the markup and what-not from the test case:

package http.voting;

import static org.junit.Assert.assertEquals;
import http.SeleniumTest;

import org.junit.Test;

import pages.IndividualRecording.IndividualRecordingPage;
import pages.song.SongPage;
import pages.voting.RecommendPanel;

public class TestUpvote extends SeleniumTest
{
    protected boolean needsDatabaseReset()
    {
        return true; 
    }
    
    @Test
    public void reproduceErrorForUserActivityAndDuplicateKeys()
    {
        // go to recording 8 
        IndividualRecordingPage recording8 = new IndividualRecordingPage(driver, 8);        
        driver.get(recording8.getURL());
        
        // upvote it
        recording8.focus(); 
        RecommendPanel votingPanel = new RecommendPanel(driver);         
        assertEquals(RecommendPanel.VotingResult.SUCCESS,  votingPanel.upvote());
        
        // go to song page 
        SongPage song257 = recording8.gotoSong(257);
        // upvote song twice
        assertEquals(RecommendPanel.VotingResult.SUCCESS,  votingPanel.upvote());
        assertEquals(RecommendPanel.VotingResult.ALREADY_VOTED, votingPanel.upvote()); 

        // go to "July 8th, 1978 - Roslyn etc.." 
        song257.gotoAssociatedRecording(26);
        // upvote (didn't work in bug)
        assertEquals(RecommendPanel.VotingResult.SUCCESS, votingPanel.upvote());         
    }
}

Minor note: the needsDatabaseReset() method tells the SeleniumTest class to restore the database to a fresh state each time a test is run.

You can see the test case deals with specific actions such as "upvote" and "go to song" instead of searching the website for different elements to click. This is all handled by the different page objects. For example, here is the relevant section of the RecommendPanel class:

     public VotingResult upvote()
    {
        final int currentVoteCount = getCurrentVoteScore(); 
        getUpvoteButton().click(); 
        
        try
        {
            // success if the votes increase or the "error box" pops up -- although we still need to check it 
            (new WebDriverWait(source, 5)).until(new ExpectedCondition<Boolean>() {
                public Boolean apply(WebDriver d) {
                    return getCurrentVoteScore() > currentVoteCount || getVisibleErrorBox() != null ; 
                }
            });
            
            int newVoteScore = getCurrentVoteScore(); 
            
            if(newVoteScore > currentVoteCount) 
                return VotingResult.SUCCESS;             
            else if(getVisibleErrorBox() != null && getVisibleErrorBox().getText().contains("You already voted"))
                return VotingResult.ALREADY_VOTED; 
            else
                return VotingResult.ERROR; 
        }
        catch(Exception exc)
        {
            exc.printStackTrace();
            return VotingResult.ERROR; 
        }
        
    }

Here, we're simply clicking the "recommend" button and ensuring that the result is either an increase in the current vote score, a popup saying "you already voted," or an error message.

With this test, the problem is now easily reproducible. The actual problem is very unexciting. We log user actions by IP address to decrease the chance of duplicate voting. The table we use is mapped to multiple classes with the id "generator" set to increment. This caused duplicate primary keys to be generated when new entries were added. Changing the generator to "identity" fixed the problem.

In contrast to the previous bug, this one was hard to reproduce but an easy fix. Stay tuned for more bugs that are both irreproducible and hard to fix!*

And for reference, the PageObjects used for this test:

IndividualRecordingPage
SongPage
RecommendPanel

* hopefully not

Wednesday, September 14, 2011

The PageObject pattern for Selenium WebDriver UI tests

There was a bug recently encountered while browsing the graphs page. Clicking on two of the canned graph options yielded this friendly result:

Woops. So at that moment I was also experimenting with Selenium WebDriver for automating some UI tests. So I figured "hey, why not reproduce the problem with some of these tests before fixing it."


At first I just stuck the Selenium test code in each test case, but after awhile I refactored to use the page object pattern. Basically you have a class that represents some part of the page (in this case, GraphsPage) which serves as an interface the the component's services, such as "generate graph." in our case.



So before the first step, it's time to define a base class called PageObject which we'll subclass for the graphs page. At this point I'll also note that Selenium seems to have some kind of support integrated called a PageFactory that you use on your page object class. In the near future I might refactor them again to use that.

But anyway, here's the PageObject:
package pages;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;

public abstract class PageObject 
{
    protected WebDriver source; 
    
    protected PageObject(WebDriver source)
    {
        this.source = source; 
    }
                
    public abstract String getURL(); 
    
    public abstract void focus(); 
}

Nothing too exciting here, it's just some boiler plate.

On to the fun part, the GraphsPage class:

package pages.graphs;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedCondition;
import org.openqa.selenium.support.ui.Select;
import org.openqa.selenium.support.ui.WebDriverWait;

import pages.PageObject;

public class GraphsPage extends PageObject
{
    private String GRAPH_URL = "graphs"; 
    
    public GraphsPage(WebDriver driver)
    {
        super(driver); 
    }
    
    private WebElement getGeneratedURLLabel()
    {
        return source.findElement(By.cssSelector("#graphpanel #graphlink")); 
    }
    
    private  Select getCannedGraphsList()
    {
        return new Select(source.findElement(By.cssSelector("#cannedgraphs select")));
    }
    
    private  WebElement getGenerateGraphsButton()
    {
        return source.findElement(By.className("getgraph"));
    }
    
    public String getURL()
    {
        return GRAPH_URL; 
    }
    
    public boolean generateGraph()
    {
        getGeneratedURLLabel().clear(); 
        getGenerateGraphsButton().click();  
        
        try
        {            
            (new WebDriverWait(source, 5)).until(new ExpectedCondition<Boolean>() {
                public Boolean apply(WebDriver d) {
                    return getGeneratedURLLabel().getText().contains("?"); 
                }
            });
            
            return true; 
        }
        catch(Exception exc)
        {
            return false; 
        }
    }
    
    public void selectCannedGraph(String cannedGraphTitle)
    {
        Select cannedGraphs = getCannedGraphsList();         
        cannedGraphs.selectByVisibleText(cannedGraphTitle); 
        cannedGraphs.getFirstSelectedOption().click(); 
    }

    @Override
    public void focus() 
    {
        getGeneratedURLLabel().click(); 
    }
}

Alright, now for the explanation. First, I'd like to explain the presence of the focus method. It seems in my experience that using FireFox 6 and IE, the click() method would sometimes fail to work unless you focused the page the first time. So that's what that does.

Here are the service methods:
  • generateGraph(): clicks the "generate graph" button, and returns true if the graph was retrieved successfully. This is done by checking for the presence of a generated URL for that specific graph. 
  • selectCannedGraph(): selects the canned graph on the canned graph list based on it's title.
You also see a few private methods, those are there mainly to abstract references to specific HTML elements and only have one reference to them.

Now we can write a test case to reproduce the problem. The problem occurs when you select the canned graph "Most played in cities," so let's write one to reproduce that:

package http.graphs;

import static org.junit.Assert.assertTrue;
import http.SeleniumTest;

import org.junit.Test;

import pages.graphs.GraphsPage;

public class TestGraphs extends SeleniumTest
{
    
    @Test
    public void testMostPlayedInCities()
    {
        testCannedGraph_helper("Most played-in cities");        
    }
    
    private void testCannedGraph_helper(String graphToCheck)
    {
        final GraphsPage page = new GraphsPage(driver);         
        driver.get(super.getPage(page.getURL())); 
        
        page.selectCannedGraph(graphToCheck); 
        assertTrue(page.generateGraph()); 
    }        
}


Nothing too exciting, you can see I've added an additional helper method so we can test other canned graphs as well. All we do here is select the canned graph, generate it, and assert that the graph was generated correctly. If you run it, it will fail.

One last thing, you can see that this test class extends SeleniumTest. That's just a boiler plate class, it looks like this:

package http;

import org.junit.After;
import org.junit.Before;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.firefox.FirefoxDriver;

public class SeleniumTest 
{
    protected WebDriver driver;
    protected final String ROOT = "http://localhost:8080/Recordings/"; 
    
    protected WebDriver getNewDriver()
    {
        return new FirefoxDriver(); 
    }
    
    @Before
    public void Before()
    {
        driver = getNewDriver(); 
    }
    
    @After
    public void After()
    {
        driver.quit(); 
    }
    
    public String getPage(String page)
    {
        return ROOT + page; 
    }
}

It just initializes the Firefox drvier and restarts it after each test. Alright, so next time I'll show you what the actual problem is and how it was fixed. Until then, take care!

Saturday, September 10, 2011

The joys of automating your build and deployment

Hi everyone,

Alright, so you know like everyone I'm sure (if not, start) I like to browse stackoverflow and it's more subjective version programmers. Take a look at this question about the benefits of nightly builds for a one man team.

Having gone all these years on my own projects without a real build server setup -- I can say that I would never hesitate to use one for a non-trivial project, whether it was just me or N other people. Now the post above is about "nightly builds," but the precursor to that is an automated build. After that, you can run it nightly or by the hour -- there's no difference.

For me, using a tool like eg. Hudson makes your life a lot more easier. It guarantees you have a "working", reproducible copy of your project and if you want ready for deployment. It allows you to do other fun things like run your unit/integration tests on SCM changes (because you will forget!), run static code analysis tools and other metrics, and see trends over time.

We (my friend and I working on Recordings) have a separate Amazon EC2 instance running Hudson 24/7 just for that purpose. Our project is (relatively) tiny and there's only two of us. We've spent some hours setting it up -- and I wouldn't hesitate to do it again. We have it doing all kinds of stuff, such as:
  • Monitoring our dev branch for changes; building and running unit tests whenever a change is detected
  • Monitoring our production branch for merges (which have been previously tested); building, running unit and integration tests, minifying js/css code, and packaging the project into a WAR file for deployment. 
  • Jobs to synchronize our local test databases with the production database
  • A job that polls the website every so often to make sure it's still online
  • A job that runs findbugs and compiles tasks (ie  TODOs) in source code
  • Lastly: a job that deploys the last successful build from production to our production server
 We use this Hudson tray tracker tool to see the status of everything at a given time, so we always know what's going on.

This allows us to be as lazy as possible, because the last thing you want to do is waste your time manually going through the build process when you're trying to fix bugs or merge new features!

And finally, as I said I find this useful even for solo projects that still have actual users* (other than yourself). Having your CI server able to build your application, create installers/docs/etc., and generate a setup file ready for distribution is very handy and will save you lots of time.


* I've used Hudson for the Recordings java webapp and some smaller C# desktop applications.

Tuesday, September 6, 2011

How transparently can you implement open session in view late in development?

On the Recordings project, we set up hibernate to eagerly load everything. I'm not sure why we made that decision (it was 8 months ago) but we've been busy getting rid of that practice. When deciding how to handle the lazy loading problem, we found a few different solutions:

  1.  Create different presentation objects for different pages and eagerly load everything into those -- pages that only need certain properties would get a class only containing those properties. 
  2. Add flags to our data access methods that indicate what to fetch
  3. Keep the db connection open while the view is being rendered (known as the Open Session in View pattern)
Approach (1) seems like a sound approach (and we've done it before in other projects) but would add too much additional maintenance and work this late in the development, as we're trying to restrict ourself to bug fixes and performance enhancements as necessary.

Approach (2) really seems ugly because not only does the data layer get convoluted -- it exposes the idea of "lazy loading" to other layers that shouldn't care about it, since they now have to decide what items they need.

So we've taken approach #3. We've implemented it by using a servlet filter that runs before any servlets. It simply opens a new session and then closes the session after the request is completed. One drawback is that if anything goes wrong with the database, the code that's responsible isn't shown in the stack trace. Instead, it shows the servlet filter as the cause which is not helpful.

A good (though painful) thing is that it's forced us to consolidate all of our data access code into one class. We have more than one class in our data layer, but they all extend one class (called BasicHibernateDataSource) which handles acquiring the session object. Our db tests also failed after doing this until we set up a basic database test class that mimics the servlet filter.

Although it seems like a lot, I think the effort required to implement it has still been "less" effort than the other two approaches, or at least the most transparent. This has allowed us to avoid introducing many new bugs due to adding a bunch of different functionality a this stage -- we're trying to push on so we can get it live as soon as possible.