Extract Implementer

In my current work of tdd’ing any new subcommander code (clean code of course :-) I want to create some test code that leads me to a class that compares the old working copy status and new working copy status of a single directory to notify only items to the gui that have changed.

I hope to achieve a much smother and faster working copy view this way. The updates will be fed to a new QAbstractItemModel implementation that provides the data for the working copy view. Currently I have about 50 tests that exercise the item model implementation and its helper classes.
I guess this is a ridiculous number of tests for a single interface implementation and a couple of helper classes!? It is not even fully implemented yet. I wonder if forcing any code line by test code is a little bit to much…
I also wonder if there are tests I could remove because “their” code (that is the production code I wrote because of the test) is now hit by other more interesting tests.

Back to wc (working copy) status. The wc status information of any single item is represented by a svn::WcStatus object. When I created the class a few years ago I didn’t want to have it a lot of setter methods. So I passed an svn_wc_status2_t* pointer to its constructor and it extracts all information from it. svn_wc_status2_t is struct from subversions api which contains all the status info.

WcStatus (const sc::String&, svn_wc_status2_t*);

Great or better not great, this is bad for testing. I don’t want to setup an svn_wc_status2_t struct just to create a couple of WcStatus objects for testing. I need a WcStatus without it. Tdd’ing the new code has to wait. First I need to break some dependency.

Typically you would do an extract interface here. Or even better, as I want to keep WcStatus as the interface name, it is used everywhere in the code, extract implementer (Working Effectivly with Legacy Code).

This is easy and straightforward:

• copy WcStatus.h to WcStatusImpl.h
• move WcStatus.cpp to WcStatusImpl.cpp
• rename the class to WcStatusImpl in WcStatusImpl.h and cpp
• #include and derive WcStatusImpl from WcStatus
• Remove the constructors and fields from WcStatus.h

No problem, easily done even without ide support. At this point it compiles again but it does not link. There is one annoying step left. You have to make all methods pure virtual in the interface. It is a bit boring to add the virtual and = 0 to 25 methods…

But with this rather small refactoring I have improved my code:

• There is now one dependency less in my old legacy code which allows me to implement WcStatus test doubles

It is only a small improvement, but it is an improvement.. :-)

Subcommander Status..

Hi,

It looks like I’m currently (currently!?) losing a bit track of things in Subcommander development… :-(

I tried to upgrade from Qt 4.4 to Qt 4.5. Bam, a couple of (my few) tests for the custom QAbstractItemModel (which is the base for the working copy display) don’t run anymore. The working copy view now crashes randomly. Great. Unfortunately I have so few test for the model, that I’m sure I will break other things trying to fix the broken tests. I’ve already wasted a lot of time debugging that code in the past with all the QModelIndex objects you can’t look into. I don’t want to waste more time on this….

So I took another step back (moving the proxy model into the TODO list again) and started another round on the QAbstractItemModel stuff. I’m now trying to TDD the model into existence. This is going a bit slow because I ‘m still a beginner at TDD. But I have about 20 tests around the new custom model implementation now and slowly it begins to do something.. :)

So far I always used cppunit to write tests. I think I’m going to drop it in favor of gtest/gmock. Allthough gmock does work together with cppunit, it is simpler and faster for me to create new tests with gtest instead of cppunit. With cppunit I always had a header and a implementation file. With gtest I just need an implementation file. This is a lot easier. Maybe this would work with cppunit too, but since gmock includes gtest anyway I have one dependency less to care for (just gmock instead of cppunit & gmock).

Another big issue that keeps me from making real progress is the maintainability of 3 build environments. Another time waster. I’m considering to move to qmake as the build environment and to drop the hand made configure/XCode/Visual Studio builds.

I also wonder if it wouldn’t be a lot easier to drop Visual Studio completely and use MinGW to build Subcommander on Windows. That way I would more or less use the same build environment (gcc and friends) on all the three platforms (Mac/Linux/Windows). This should improve maintainability as well.

So I’m still working on Subcommander, but I can’t tell when there will be a new release… but maybe I’m the only Subcommander  user anyway… ;)

creating a custom QAbstractProxyModel – Google Mock

Part I redone, getting started using Google Mock.

A while ago I started to build a custom QAbstractProxyModel for subcommanders working copy view in creating a custom QAbstractProxyModel.

A few weeks ago I red about Google Mock, a mock framework for C++ and I planned to try it. There is a nice, simple and short introduction: Google Mock for Dummies.

In the last creating a custom QAbstractProxyModel article I created a simple mock from scratch that is simple enough for a first try. So the goal is to simplify that. If it doesn’t simplify that code, why would I want to use a mock framework? :)

Here is the original hand made mock class and test. Constructor and destructor connect and disconnect the signals from src (that is the proxy model) to the slots and verify is used to verify the results.

class SignalTarget : public QObject
{
  Q_OBJECT;

public:
  SignalTarget( QObject* src );
  ~SignalTarget();

  virtual void verify();

public slots:
  void modelAboutToBeReset();
  void modelReset();

private:
  QObject* _src;

protected:
  int  _hitModelAboutToBeReset;
  int  _hitModelReset;
};

To check that the tested class (the proxy model) properly emits the signals, I subclassed SignalTarget to ASSERTS that the slots were called as many times as they should. Here is the test code:

void WcViewItemProxyModelTest::emitsModelResetSignals()
{
  class Target : public SignalTarget {
  public:
    Target( QObject* src ) : SignalTarget(src) {
    }

    void verify() {
      CPPUNIT_ASSERT_EQUAL( 1, _hitModelAboutToBeReset );
      CPPUNIT_ASSERT_EQUAL( 1, _hitModelReset );
    }
  };

  Target target(_proxy);
  _proxy->reset();
  target.verify();
}

Using Google Mock this gets a bit simpler. The basic mock class get a bit shorter because I no longer need the verify method and the int counters. Google Mock will take care of it:

class SignalTarget : public QObject
{
  Q_OBJECT; 

public:
  SignalTarget( QObject* src );
  ~SignalTarget(); 

public slots:
  void modelAboutToBeReset();
  void modelReset(); 

private:
  QObject* _src;
};

To a create the actual mock class I derive a new class from SignalTarget and tell it to mock the ...Reset() methods by using the MOCK_METHODx macro:

class MockSignal : SignalTarget
{
public:
  MockSignal(QObject* target) : SignalTarget(target) {
  }

  MOCK_METHOD0(modelAboutToBeReset, void());
  MOCK_METHOD0(modelReset, void());
};

Finally I create the mock object in the test and set the expectations (the methods should be called 1 time) by using the EXPECT_CALL macro and calling Times with the expected number of calls. This is a lot simpler than creating the mock class in the original code to set up the expectations in the verify() method.

void WcViewItemProxyModelTest::emitsModelResetSignals()
{
  MockSignal mtarget(_proxy);
  EXPECT_CALL(mtarget, modelAboutToBeReset()).Times(1);
  EXPECT_CALL(mtarget, modelReset()).Times(1);

  _model->reset();
}

All in all it was quite easy to get started with Google Mock. Although I used it only on a very simple example I think it is a nice help for writing tests. If it fits the thing you want to test it can reduce the code noise required to set up the test.

If you write tests in c++, check it out.. :-)

creating a custom QAbstractProxyModel

Part I, getting started.

I’m currently implementing a custom QAbstractProxyModel for subcommanders working copy view. The first reason is that I want to improve the status filtering in subcommander which is not possible with the current code based on QSortFilterProxyModel. The second reason is to get full control of sorting. My attempt to add sorting to the current code failed in such a frustrating way that I don’t want to track down the problem and simply start again with fresh code that I will be able to debug :-)

Before starting with the interesting stuff (filtering and sorting) I will create a simple proxy model which does not do any sorting or filtering. It will just pass through the original (tree) model without modification. Getting this running is a big step toward the final goal.

Important for a proxy model is that is emits all the signals the source or original model emits. To achieve this the proxy model must connect to all (interesting) signals from the source model and emit the same signals with proper information from the proxy model.

I have created a test checking two signals emitted from the proxy model. To count the emitted signals I’m using a SignalTarget class which connects to the proxy signals and checks if the signals were emitted. Since the verification (CPPUNIT_ASSERT) is done inside this class it is a mock. It replaces the widget (in my case a QTreeView) that will be connected to the proxy model in the production code.

class  SignalTarget : public  QObject
{
  Q_OBJECT; 

public:
  SignalTarget( QObject* src );
  ~SignalTarget(); 

  virtual void verify(); 

public  slots:
  void modelAboutToBeReset();
  void modelReset(); 

private:
  QObject* _src; 

protected:
  int  _hitModelAboutToBeReset;
  int  _hitModelReset;
};
 

The idea is to count the emitted signals and check the results in the verify method. It is not yet complete. It does handle only a subset of the signals we need to check. I have declared verify as virtual so it easy to re-implement and do the actual check. The default implementation is empty. The constructor and destructor will simply connect and disconnect all signals the we need to handle in the proxy model. The slot method names correspond to the model signals.

A test method can now use SignalTarget to check the signals from the proxy model. You can see the complete test below. _proxy is the proxy model and gets created by the test setup. The reset() method triggers both signals. Because I care only for the reset signals I have overwritten verify() to simply check their call counts.

void WcViewItemProxyModelTest::emitsModelResetSignals()
{
  class Target : public SignalTarget {
  public:
    Target( QObject* src ) : SignalTarget(src) {
    }

    void verify() {
      CPPUNIT_ASSERT_EQUAL( 1, _hitModelAboutToBeReset );
      CPPUNIT_ASSERT_EQUAL( 1, _hitModelReset );
    }
  };

  Target target(_proxy);
  _proxy->reset();
  target.verify();
}
 

This is similar to the signal testing I mentioned in my last article. The only difference is that originally I also tried to check QModelIndex indices which didn’t work to well. The problem was creating valid expected indices to assert on. I tried to remember indices from previous signals and build new expected indices (sibling, parent,child) from them. Unfortunately this approach got complicated rather fast so you couldn’t follow the code anymore.

To get the test running I simply connect the soure model modelReset signal to the srcModelReset() slot in the proxy model and call reset(). reset() will emit both signals the test wants to see.

void WcViewItemProxyModel::srcModelReset()
{
  reset();
}
 

Very simple so far, but one has to start somewhere. :-) I guess it will get more interesting for some of the other signals the proxy model has to handle. More on this in the next article.

QAbstractItemModel again..

I have implemented subcommanders working copy view as a custom QAbstractItemModel (since subcommander displays a directory tree it is a tree model). On top of the model is a proxy model based on QSortFilterProxyModel to filter working copy items based on their subversion status.

I have a mixed feeling about Qt4’s item models. It has some benefits but also a few problems.

On the positive side is the view and model separation, which of course is good. The Qt item model is really a view model and not simply a data model. But that’s not a problem, you just have to accept that there a few view things you have to handle in the model.

Another useful feature is the proxy model. As already said I use it to filter items by their working copy status and I also use it to realize part of the flat and the tree view display. Switching between both is a matter of toggling an option in the proxy model and tweaking a few options in the widget that is connected to my model.

The bad thing about the item model stuff it that it is really complicated to get it working properly. Apart from all the documentation that Qt provides on implementing custom models, it is still difficult to get all the QModelIndex stuff running.

Another issue is that it is really hard to debug. You may have a small bug in your model that crashes the application. Now debug that. :-( The issue is that you can’t directly watch the data behind a QModelIndex. So it is unfortunately very frustrating to find out what’s wrong.

From my point of view, item models are too complicated. Said that, it is what you have to use in Qt4 if the simpler model stuff doesn’t work for you. And it doesn’t work for me.

So what can I do?

A while ago I was trying to find a bug in my custom QAbstractItemModel. I couldn’t track it down with the debugger and finally started to write a couple of unit test for it using cppunit. (Yes, Qt has a unit test framework that supports signals and slot, but I’m already using ccpunit and I don’t want to have tests in different frameworks :-)

I didn’t find this bug with the tests but it helped me with another signal issue in the custom model. Under certain data constellations a change in the underlying item model wasn’t properly notified to the proxy model.

I have tried two approaches. The first checks for emitted signals from the model when inserting data into the model. The second checks the existence of index values which should exist after inserting data into the model.

I think I prefer the second approach, but I’m not yet satisfied with the test code itself. It is large and also a bit messed up…

I will describe both ways in the next articles and also write about my experience with creating a custom QAbstractProxyModel.