Page 6 of 15

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 7:39 am
by arjan.top

Code: Select all

 
  public function testInstantiateDatastoreLoadsStorageModel(){
    $this->_storageModel->expectOnce('load');
    $dataStore = new DataStore($this->_storageModel);
  }
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 7:45 am
by matthijs
And how would you make that pass?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 7:59 am
by arjan.top
Looks like that won't work ...

Code: Select all

 
  public function testInstantiateDatastoreLoadsStorageModel(){
    $storageModel = new MockStorageModel();
    $storageModel->expectOnce('load');
    $dataStore = new DataStore($storageModel);
  }
 

Code: Select all

 
  public function __construct(StorageModel $storageModel) {
    $this->_storageModel = $storageModel;
    $this->_storageModel->load();
  }
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:15 am
by Chris Corbyn
Create factory methods for the objects so we keep that looser coupling:

Code: Select all

class DataStoreTest ...
 
  public function setUp() {
    $this->_storageModel = $this->_createStorageModel();
    $this->_dataStore = $this->_createDataStore($this->_storageModel);
  }
 
  ....
 
  public function testInstantiateDatastoreLoadsStorageModel() {
    $storageModel = $this->_createStorageModel();
    $storageModel->expectOnce('load');
    $this->_createDataStore($storageModel);
  }
  
  ....
 
  private function _createStorageModel() {
    return new MockStorageModel();
  }
 
  private function _createDataStore(StorageModel $storageModel) {
    return new DataStore($storageModel);
  }
 
}
On a side note, from what I remember just calling $this->_storageModel->expectOnce('load') would have worked since tally() doesn't get called until after the entire test method has run. This is a bit of a weird point to make though so don't pay much attention to me...

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:24 am
by Chris Corbyn
Next test. Make sure that if load() returns something useful we hydrate our DataStore with it's values.

Code: Select all

class DataStoreTest ...
 
   ...
  
  public function testDataStoreLoadsValuesFromStorageModel() {
    $storageModel = $this->_createStorageModel();
    $storageModel->setReturnValue('load', array('zip' => 'button'));
    $dataStore = $this->_createDataStore($storageModel);
    $this->assertEqual('button', $dataStore->get('zip'),
      '%s: Values from StorageModel should be loaded into DataStore'
      );
  }
Notice here that I've passed a custom expectation message. The %s allows SimpleTest to add it's standard error, then the following text is appended. It means that when our test fails we have a little description of why it's failing. I like to try and use this as much as possible... but in practise you never do it often enough.

PS: You'll probably pick up my weak points soon. I'm terrible at writing descriptive method names which aren't as long as my arm! BDD helps here but it's not something I'm really qualified to talk about yet ;)

Code: Select all

AllTests.php
1) Equal expectation fails at character 0 with [button] and []: Values from StorageModel should be loaded into DataStore at [/Users/d11wtq/data_store/tests/unit/DataStoreTest.php line 60]
    in testDataStoreLoadsValuesFromStorageModel
    in DataStoreTest
    in /Users/d11wtq/data_store/tests/unit/DataStoreTest.php
    in All DataStore tests
FAILURES!!!
Test cases run: 1/1, Passes: 7, Failures: 1, Exceptions: 0

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:25 am
by matthijs
So basically by adding those private functions and calling them in testInstantiateDatastoreLoadsStorageModel, you re-instantiate the DataStore and StorageModel classes, so that the constructor can be tested?

[edit] didn't see your second post. This one is about the one before that. First I'm getting some more coffee before going on and getting brain damage

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:33 am
by Chris Corbyn
matthijs wrote:So basically by adding those private functions and calling them in testInstantiateDatastoreLoadsStorageModel, you re-instantiate the DataStore and StorageModel classes, so that the constructor can be tested?
That's right. Most of the time the SUT can be tested in it's already instantiated state. But when you're testing something which happens during instantiation then factory methods are a good trade-off. Again, there's not seriously wrong with just skipping using setUp() and/or using factory methods.... but all those calls to "new Xyz()" make refactoring harder.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:38 am
by matthijs
Two quick questions: what is SUT?

And what did you do in the previous test to make it pass? This?

Code: Select all

 
    public function __construct(StorageModel $storageModel) {
        $this->_storageModel = $storageModel;
        $this->_storageModel->load();
    }
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:41 am
by Chris Corbyn
matthijs wrote:Two quick questions: what is SUT?
System Under Test ;) It's a commonly used abbreviation to generalize anything which is being tested. Just makes it easier to talk about the "test" and the "SUT".
And what did you do in the previous test to make it pass? This?

Code: Select all

 
    public function __construct(StorageModel $storageModel) {
        $this->_storageModel = $storageModel;
        $this->_storageModel->load();
    }
 
I did, I've attached my updated files (including the new failing test we have to write code for :)).

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 8:56 am
by matthijs
I hereby officially declare Chris the most helpful member of 2008. Congratulation's Chris :)

And what happened with this test (maybe without the double assert):

Code: Select all

 
    public function testRemoveValueRemovesValue(){
        $this->_dataStore->set('foo', 'bar');
        $this->assertTrue($this->_dataStore->has('foo'));
        $this->_dataStore->remove('foo');
        $this->assertFalse($this->_dataStore->has('foo'));
    }
 

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 9:03 am
by arjan.top
ugly ...

Code: Select all

 
  public function __construct(StorageModel $storageModel) {
    $this->_storageModel = $storageModel;
    
    if(is_array($this->_storageModel->load()))
        $this->_store = $this->_storageModel->load();
  }
 
if there is no if testDataStoreKnowsWhenItDoesntHaveAValue fails

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 9:07 am
by matthijs
Yes, I found out as well. That's weird.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 9:10 am
by arjan.top
no it's not :)

You assign $this->_storageModel->load() to $this->_store, but $this->_storageModel->load() in setUp has no return value so $this->_store is no longer an array :wink:

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 9:12 am
by Chris Corbyn
arjan.top wrote:ugly ...

Code: Select all

 
  public function __construct(StorageModel $storageModel) {
    $this->_storageModel = $storageModel;
    
    if(is_array($this->_storageModel->load()))
        $this->_store = $this->_storageModel->load();
  }
 
if there is no if testDataStoreKnowsWhenItDoesntHaveAValue fails
I agree it's ugly... we can come back to it later. But maybe this is an improvement?

Code: Select all

public function __construct(StorageModel $storageModel) {
  $this->_store = (array) $storageModel->load();
  $this->_storageModel = $storageModel;
}
PS: The files I attached before we're wrong. I had not implemented the remove() stuff :oops:

New files attached.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri Apr 25, 2008 9:22 am
by matthijs
arjan.top wrote:no it's not :)

You assign $this->_storageModel->load() to $this->_store, but $this->_storageModel->load() in setUp has no return value so $this->_store is no longer an array :wink:
Smurf, I was trying to lessen my caffeine ingestion (they say it's not healthy) but obviously that was not a good decision :)

@chris: it's ok with me. It returns a green bar and that's the whole issue isn't it?