How should I unit test a function that uses setters?

Question

I'm using a repository pattern design and I've hit a stumbling block when writing a unit test for one of my methods. I'm fairly new to writing unit tests, so I would appreciate any help!

Let's say I have a method which creates a product and uses setters to set the data.

public function addProduct(array $data)
{
    $new = $this->repository->make(); // returns new Product object

    $new->setTitle($data['title']);
    $new->setPrice($data['price']);
    $new->setImage($data['image']);
    $new->setStock($data['stock']);

    return $this->repository->save($new); // returns bool
}

Now when I'm writing unit tests for this method, what should I actually be testing?

Should I just check the return type and leave it at that?
Should I mock the return of make() and ensure that all the setters were run?
Should I mock just the repository and ensure that make() and save() were run?

Initially, I decided to go with all three options. However, my concern came from when I started option 2, writing tests to ensure that all the setters were run

Should the unit test really be concerned about whether all the setters ran? What happens if I add more fields? It seems like doing this would mean the tiniest change could result in the unit test failing when the method actually does as performed.

This is how I've wrote my unit test so far, but I'm really unsure about how strict it is

public function testAddProductAddsProductWithCorrectAttributes()
{
    $newMock = Mockery::make(ProductInterface::class)
                    ->shouldReceive('setTitle')
                    ->withArgs(['Test title'])
                    ->shouldReceive('setPrice')
                    ->withArgs([10.99])
                    ->shouldReceive('setImage')
                    ->withArgs(['/foobar.jpg'])
                    ->shouldReceive('setStock')
                    ->withArgs(['In Stock']);

    $repoMock = Mockery::make(RepositoryInterface::class)
                    ->shouldReceive('make')
                    ->andReturn($newMock)
                    ->shouldReceive('save')
                    ->withArgs([$newMock])
                    ->andReturn(true);

    $service = new Service($repoMock);

    $add = $service->addProduct([
        'title' => 'Test title',
        'price' => 10.99,
        'image' => '/foobar.jpg',
        'stock' => 'In Stock'
    ]);

    $this->assertTrue($add);
}

Possible duplicate of [Where is the line between unit testing application logic and distrusting language constructs?](https://softwareengineering.stackexchange.com/questions/322909/where-is-the-line-between-unit-testing-application-logic-and-distrusting-languag) — gnat, May 03 '17 at 08:42
see also: [Is it worth writing a unit test for a DTO with the most basic getter/setters?](https://softwareengineering.stackexchange.com/questions/205716/is-it-worth-writing-a-unit-test-for-a-dto-with-the-most-basic-getter-setters) — gnat, May 03 '17 at 08:42

score 7 · Accepted Answer · answered May 03 '17 at 09:04

7

When testing a repository the essential thing to test is whether you can get out what you put in.

so... (excuse pseudo code)

$data = ..//whatever
addProduct(array $data)
$actual = getProduct(id)

Assert $actual == $data

This will obviously fail if the setters don't work. But doesn't explicitly test every setter

answered May 03 '17 at 09:04

Ewan

70,664
5
76
161

1

See, this would be my preferred method of testing - run the add function, try and get it. But I'm not persisting my data in the unit tests, so I'm unsure of how I'd be able to retrieve it – Dan Johnson May 03 '17 at 09:06
I officially pronounce your preferred way of doing it to be the best and most correctest way :) (be careful of default values) – Ewan May 03 '17 at 09:07
But wouldn't that be considered an integration test? It's relying on both `addProduct()` *and* `getProduct()` to be working as intended – Dan Johnson May 03 '17 at 09:18
I did answer another question about what order to write read/write tests so you would catch the failure of either with a specific test. essentially you need to set the database up directly. But either way you still want a 'I read back what I wrote' test at some point – Ewan May 03 '17 at 10:23
2

well if you deliberately dont persist your data i your test, even to a mocked datasource, then you are limiting what you can test. Sometimes micro tests have value, but its expensive to write then – Ewan May 03 '17 at 10:26
@DanJohnson why don't you write a test data-store that simply stores the data in an array or something? – user253751 May 03 '17 at 12:07
@immibis I guess I'll go down that path. I'm injecting all my dependencies so it would be easy enough to switch to another implementation during testing. And I could read the data directly rather than relying on the `getProduct()` method. Thanks guys! – Dan Johnson May 03 '17 at 12:44

score 3 · Answer 2 · answered May 03 '17 at 08:41

The purpose of addProduct() is to ensure that (a) a new product exists and (b) it has the field values specified by the arguments. Therefore, yes, the unit test absolutely should care whether all the setters ran and whether those are all the attributes of the object.

If it didn't care, then you would very likely introduce defects when the set of attributes changes. As it is, the test suite warns you automatically immediately after you introduce a new attribute. Believe me, annoying notifications are way better than unnoticed defects.

I've edited the question to include how I'm currently checking if the setters were called. Do you think that is the best way of going around it? — Dan Johnson, May 03 '17 at 09:17

How should I unit test a function that uses setters?

2 Answers2