Finishing Up With a Tested Wallpaper Update

By the end of the previous video we had a tested UploadedFileType to work from. Our next task is to incorporate the necessary changes from our untested prototype approach into our UploadedFileType in order to enable the image preview facility.

We need to update our buildView test to track another addition to the $view->vars array:

    public function buildView(FormView $view, FormInterface $form, array $options)
    {
        /**
         * @var $entity Wallpaper
         */
        $entity = $form->getParent()->getData();

        if ($entity) {
            $view->vars['file_uri'] = (null === $entity->getFilename())
                ? null
                : '/images/' . $entity->getFilename()
            ;
        }
    }

Our test so far:

    function it_can_build_a_view(
        FormInterface $form
    ) {
        $view = new FormView();

        $view->vars['full_name'] = 'quidditch';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'quidditch[file]'
        );
    }

We are mocking $form, so we need to figure out what a call to getParent will return, and then create a mock for that also.

On that mock we will make sure that a call to getData returns a Wallpaper instance.

How do we figure out what a call to $form->getParent() will return?

Well, we know that $form is an instance of FormInterface, so let's look at the interface itself and hopefully the documentation will give us a clue:

<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Form;

use Symfony\Component\Form\Exception\TransformationFailedException;

/**
 * A form group bundling multiple forms in a hierarchical structure.
 *
 * @author Bernhard Schussek <bschussek@gmail.com>
 */
interface FormInterface extends \ArrayAccess, \Traversable, \Countable
{
    // * snip *

    /**
     * Returns the parent form.
     *
     * @return self|null The parent form or null if there is none
     */
    public function getParent();

Ok cool, we can see that a call to getParent returns self, or in other words, and instance of FormInterface. Remember, Symfony's form is a composite of one or more forms - this is why it's quite confusing to initially understand, but conversely extremely flexible and powerful.

Even though we have our test set up to inject a FormInterface mock already, we don't want to re-use the same variable in our test when calling getParent. Let's update our test accordingly:

    function it_can_build_a_view(
        FormInterface $parent,
        FormInterface $form
    ) {
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'quidditch';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'quidditch[file]'
        );
    }

There's no change to this test in terms of outcome. We have just changed the setup process.

Next we know that we will call $form->getParent()->getData();, so let's make sure a call to getData returns a Wallpaper instance.

Thinking about it, we'd likely want to test the unhappy path here, too.

What if the returned value from getData were not a Wallpaper? What then? Good, our tests are making us think about our code's outcomes more thoroughly than before.

Let's take a moment to plan out our tests. As long as we simply write out the test name, PhpSpec will still run our existing tests for us just fine, and any empty methods will be considered "pending examples". This gives us a good idea of how long we are from being confidently done.

<?php

namespace spec\AppBundle\Form\Type;

use AppBundle\Entity\Category;
use AppBundle\Entity\Wallpaper;
use AppBundle\File\SymfonyUploadedFile;
use AppBundle\Form\Type\UploadedFileType;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;
use Symfony\Component\Form\Extension\Core\Type\FileType;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\Form\Test\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Webmozart\Assert\Assert;

class UploadedFileTypeSpec extends ObjectBehavior
{
    // * snip *

    function it_can_build_a_view(
        FormInterface $parent,
        FormInterface $form
    ) {
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'quidditch';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'quidditch[file]'
        );
    }

    function it_will_not_set_a_file_uri_view_var_if_parent_form_data_is_not_a_wallpaper()
    }

    function it_will_set_a_file_uri_view_var_of_null_if_wallpaper_has_no_filename() {

    }

    // * snip *

The first test is the happy path. Let's signifiy that:

    function it_can_build_a_view_happy_path(
        FormInterface $parent,
        FormInterface $form
    ) {

The other two tests will cover our anticipated unhappy paths. This should give us a decent amount of code coverage for this class. More importantly is the confidence it gives us in knowing that the system will respond in an expected fashion even when things do not quite go to plan.

Thinking on with our happy path, let's expand the test out:

<?php

namespace spec\AppBundle\Form\Type;

use AppBundle\Entity\Category;
use AppBundle\Entity\Wallpaper;
use AppBundle\File\SymfonyUploadedFile;
use AppBundle\Form\Type\UploadedFileType;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;
use Symfony\Component\Form\Extension\Core\Type\FileType;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\Form\Test\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Webmozart\Assert\Assert;

class UploadedFileTypeSpec extends ObjectBehavior
{
    // * snip *

    function it_can_build_a_view_happy_path(
        FormInterface $parent,
        FormInterface $form,
        Wallpaper $wallpaper
    ) {
        $wallpaper->getFilename()->willReturn('fake-file.jpg');
        $parent->getData()->willReturn($wallpaper);
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'quidditch';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'quidditch[file]'
        );
    }

    function it_will_not_set_a_file_uri_view_var_if_parent_form_data_is_not_a_wallpaper()
    {
    }

    function it_will_set_a_file_uri_view_var_of_null_if_wallpaper_has_no_filename()
    {
    }

    // * snip *

The three important lines are:

        $wallpaper->getFilename()->willReturn('fake-file.jpg');
        $parent->getData()->willReturn($wallpaper);
        $form->getParent()->willReturn($parent);

$form has a method called getParent.

Because our form type set up in EasyAdminBundle's config will set up our UploadedFileType as a child form - and at this point we are in that UploadedFileType - we must go up to the parent form to get the form's underlying data object.

This should always be an instance of Wallpaper.

We are going to have a test that validates our assumption of what happens should this ever happen not to be an instance of Wallpaper inside our pending test:

it_will_not_set_a_file_uri_view_var_if_parent_form_data_is_not_a_wallpaper

As $parent is another instance of FormInterface, it too has all the methods of a form. We can therefore call getData to get back the underlying data. In our case this will be an object of type Wallpaper.

We mock the Wallpaper instance for simplicity. We could just as easily use a real, new object.

We set up the test to reply with a value of fake-file.jpg whenever a call to getFilename is made against our mocked Wallpaper.

This setting up of our test object's nested hierarchy was one of the most confusing parts of learning testing for me.

Finally, we should add in an assertion for what we expect on our $view->vars if this all goes to plan:

    function it_can_build_a_view_happy_path(
        FormInterface $parent,
        FormInterface $form,
        Wallpaper $wallpaper
    ) {
        $wallpaper->getFilename()->willReturn('fake-file.jpg');
        $parent->getData()->willReturn($wallpaper);
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'quidditch';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'quidditch[file]'
        );

        Assert::same(
            $view->vars['file_uri'],
            '/images/fake-file.jpg'
        );
    }

This does highlight a overly concrete element of our code: /image/fake-file.jpg.

For now, I am happy to leave this brittleness in here. Our tests cover it, and should help us reliably fix it as and when our time comes to expand out to S3 (or similar).

This test fails, as is expected, because we don't currently have any logic in buildView that would set a file_uri.

We can re-use the existing implementation from our untested approach here:

<?php

namespace AppBundle\Form\Type;

use AppBundle\Entity\Wallpaper;
use AppBundle\File\SymfonyUploadedFile;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\FileType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\OptionsResolver\OptionsResolver;

class UploadedFileType extends AbstractType
{
    // * snip *

    public function buildView(FormView $view, FormInterface $form, array $options)
    {
        $view->vars['full_name'] = $view->vars['full_name'] . '[file]';

        /**
         * @var $entity Wallpaper
         */
        $entity = $form->getParent()->getData();

        if ($entity) {
            $view->vars['file_uri'] = (null === $entity->getFilename())
                ? null
                : '/images/' . $entity->getFilename()
            ;
        }

    }

    // * snip *

And now our test should be passing.

There's two further checks to be made here: our pending examples.

Let's start with what happens if the returned value of getData were somehow not to be a Wallpaper instance.

    function it_will_not_set_a_file_uri_view_var_if_parent_form_data_is_not_a_wallpaper(
        FormInterface $parent,
        FormInterface $form,
        Category $category
    ) {
        $parent->getData()->willReturn($category);
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'bob';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'bob[file]'
        );
    }

The setup here is roughly the same, just a little simpler.

We don't need to fake out the return to a call to getFilename, as we should never make that call.

We also need to an assertion here that file_uri is not set.

There's likely a few ways to cover this scenario. Here's mine:

    function it_will_not_set_a_file_uri_view_var_if_parent_form_data_is_not_a_wallpaper(
        FormInterface $parent,
        FormInterface $form,
        Category $category
    ) {
        $parent->getData()->willReturn($category);
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'bob';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'bob[file]'
        );

        Assert::false(
            isset($view->vars['file_uri'])
        );
    }

This should pass, right?

php vendor/bin/phpspec run spec/AppBundle/Form/Type/UploadedFileTypeSpec.php

AppBundle/Form/Type/UploadedFileType
  60  - it will not set a file uri view var if parent form data is not a wallpaper
      method `Double\AppBundle\Entity\Category\P3::getFilename()` not found.

                                      100%                                       1
1 specs
1 example (1 broken)
50ms

Alas, no.

What this highlights is a bug in our original implementation.

        /**
         * @var $entity Wallpaper
         */
        $entity = $form->getParent()->getData();

        if ($entity) {

Here getData is returning something (a Category), so simply checking if $entity is set is not enough.

Worse, the use of the @var annotation is confirming what we expect to happen, even though it might not be the case.

Fortunately fixing this is as simple as adding in an instanceof guard statement:

        /**
         * @var $entity Wallpaper
         */
        $entity = $form->getParent()->getData();

        if ($entity instanceof Wallpaper) {

Now the test passes, as we won't be getting past that line when our $entity is an instance of Category, so a call to getFilename is never made.

What about our other anticipated problem area?

it_will_set_a_file_uri_view_var_of_null_if_wallpaper_has_no_filename

That's quite the function name.

    function it_will_set_a_file_uri_view_var_of_null_if_wallpaper_has_no_filename(
        FormInterface $parent,
        FormInterface $form,
        Wallpaper $wallpaper
    ) {
        $wallpaper->getFilename()->willReturn(null);
        $parent->getData()->willReturn($wallpaper);
        $form->getParent()->willReturn($parent);

        $view = new FormView();

        $view->vars['full_name'] = 'qwerty';

        $this->buildView($view, $form, []);

        Assert::same(
            $view->vars['full_name'],
            'qwerty[file]'
        );

        Assert::null($view->vars['file_uri']);
    }

There are two important lines in this test. Almost everything is as the happy path test except:

$wallpaper->getFilename()->willReturn(null);

This is to cover:

        $entity = $form->getParent()->getData();
        if ($entity) {
            $view->vars['file_uri'] = (null === $entity->getFilename())
                ? ...
                : ...
            ;
        }

Specifically to cover the outcome of the null === $entity->getFilename() check.

In this instance, null will be null, which is true, so the truthy part of our ternary is used.

This will happen when we visit the form in Create mode.

We need to validate that whilst the key of $view->vars['file_uri'] is set, its value is set to null:

Assert::null($view->vars['file_uri']);

Does this pass?

Yes.

We're getting there.

We need to update our configureOptions test to set a default value of the file_uri option to null.

This is a nice and easy one:

    function it_correctly_configures_the_expected_options(
        OptionsResolver $resolver
    )
    {
        $this->configureOptions($resolver);

        $resolver
            ->setDefaults([
                'data_class' => SymfonyUploadedFile::class,
                'file_uri'   => null,
            ])
            ->shouldHaveBeenCalled()
        ;
    }

And the implementation:

    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver
            ->setDefaults([
                'data_class' => SymfonyUploadedFile::class,
                'file_uri'   => null,
            ])
        ;
    }

And lastly we need to update our uploaded_file_widget to use the new variable:

<!-- /app/Resources/views/Form/image_upload.html.twig -->

{%- block uploaded_file_widget -%}
    <input type="file" {{ block('widget_attributes') }}/>

    <!-- image preview -->
    {% if file_uri is defined and file_uri is not null %}
        <img src="{{ file_uri }}" class="img img-responsive img-preview thumbnail" />
    {% endif %}
{%- endblock uploaded_file_widget -%}

And with that, we have a working, tested image preview facility.

I said at the start of the previous video that I'd cover what this test does not cover.

Primarily it does not test this template.

In order to do so you would likely need to rely on Acceptance Tests. In order to do this, one solution maybe to rely on Codeception or similar. Behat can also do this.

However, from personal experience, Acceptance testing in this way leads to brittle tests. It is extremely easy to tie your tests to the graphical elements of your website. If these change - sometimes even just slightly - the tests break in weird and potentially unexpected ways. Also, these tests are slow.

As ever it is a trade off. If you want to ensure the stuff that end users see visibly behaves as expected, you have to take an approach similar to that described above.