5

I am contributing to sonata/exporter, a library used for export data in many formats (CSV, JSON, XML, XLS, ...).

I work on a Writer that converts boolean values into strings (e.g. yes/no) by encapsulating another Writer (like CsvWriter or XlsWriter).

It's my first experience with phpunit.

All unit tests made on the existing Writers use this logic :
- Create a file.
- Write data in file using the corresponding format.
- Make an assertEquals on file_get_contents(filename).

So, I've written this test : 

public function setUp()
{
    $this->filename = 'formatedbool.xls';
    $this->sampleWriter = new XlsWriter($this->filename, false);
    $this->trueLabel = 'oui';
    $this->falseLabel = 'non';

    if (is_file($this->filename)) {
        unlink($this->filename);
    }
}

public function testValidDataFormat()
{
    $writer = new FormatedBoolWriter($this->sampleWriter, $this->trueLabel, $this->falseLabel);
    $writer->open();
    $writer->write(array('john', 'doe', false, true));
    $writer->close();

    $expected = '<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8" /><meta name=ProgId content=Excel.Sheet><meta name=Generator content="https://github.com/sonata-project/exporter"></head><body><table><tr><td>john</td><td>doe</td><td>non</td><td>oui</td></tr></table></body></html>';
    $this->assertEquals($expected, trim(file_get_contents($this->filename)));
}

When submitting my PR, the owner says me :

just use a mock with expected method call and check calling argument, this will avoid creating the file. see https://phpunit.de/manual/current/en/test-doubles.html#test-doubles.mock-objects.examples.with-consecutive.php

I have begin to rewrite tests using a Mock but I have an error on file_get_contents because file is not created.

The "write" method just write in a file and return nothing.

I think he want I test the data after converting bools, but before writing in file. How can I check the result of the file content without really create it ? Or simply access to my $data during the method call ?

EDIT Thanks to @Cerad, the code I've submitted :

public function testValidDataFormat()
{
    $data = array('john', 'doe', false, true);
    $expected =  array('john', 'doe', 'no', 'yes');
    $mock = $this->getMockBuilder('Exporter\Writer\XlsWriter')
                   ->setConstructorArgs(array('formattedbool.xls', false))
                   ->getMock();
    $mock->expects($this->any())
           ->method('write')
           ->with($this->equalTo($expected));
    $writer = new FormattedBoolWriter($mock, $this->trueLabel, $this->falseLabel);
    $writer->open();
    $writer->write($data);
    $writer->close();
}

I'm waiting for answer of the project owner.

EDIT PR merged at https://github.com/sonata-project/exporter/pull/56

chalasr
  • 12,971
  • 4
  • 40
  • 82
  • 1
    He does not want you to mock the FormattedBoolWriter, after all that is the class being tested. It is the sample writer that needs to be mocked. You then intercept whatever call your BoolWriter makes to the same writer and verify it has the expected values. – Cerad Nov 19 '15 at 14:33
  • I've edited my question and rewritten my tests. Can you take a look please ? Thank's for your help. – chalasr Nov 19 '15 at 15:48
  • 1
    You should not test how many times the data is written to the file, because you care only if the data is written, so you can remove `->expects($this->once())` (_for phpunit < 4 you can use `->expects($this->any())`_) and your test should still be green. – Alexandru Guzinschi Nov 19 '15 at 16:01
  • 1
    Yep. Looks good. And yeah the once parameter is probably not strictly needed but doesn't really hurt. Submit and see what the owner has to say. Let us know if it gets accepted. – Cerad Nov 19 '15 at 16:05
  • Thanks a lot for your help ! I have submitted changes and a collaborator of the project has asked me for add phpdoc on Writer and test members. Nothing about the testing code, it seems to be ok. I'll let you know – chalasr Nov 19 '15 at 19:44
  • @Cerad my PR has been successfully merged this morning. Thank you a lot for your help. https://github.com/sonata-project/exporter/pull/56 – chalasr Nov 20 '15 at 09:58

1 Answers1

-1

This question has been answered by @Cerad by commenting on the question.

The PR has been accepted and merged, see https://github.com/sonata-project/exporter/pull/56

chalasr
  • 12,971
  • 4
  • 40
  • 82