DMA's avatar
Level 2

Local variable overwritten in test - how?!

It's been a long day, and I must be missing something glaringly obvious so I'm hoping that someone here can point out what's going on.

I have a simple unit test which is testing that a class returns an unmodified object after doing logic.

Quite simply, my question is - how is the $user object's name attribute being changed?

To give more context:

Given there is a User named 'Bob Jones'

 $user = factory(App\User::class, 'withCompany')
            ->make([
                'name' => 'Bob',
                'member_id' => 1
            ]);

And Bob has some meta UserData which also has attributes name and surname (of equal values).

 $userData = factory(App\UserData::class)
            // And the data is the same
            ->make($this->mapUserToUserData($user) + ['member_id' => 1]);

When I change the name attribute within the UserData meta data object to 'Fred',

$userData->m_field_id_4 = 'Fred';

And I synchronise the data between objects

$this->syncUserData->sync($user, $userData);

Note: At this point - if I dd($user->name); 'Fred' is output, instead of 'Bob', as I would expect.

Then the User object should still have the name 'Bob'.

$originalUser = $this->syncUserData->getOriginalUser();
$syncedUser = $this->syncUserData->getSyncedUser();

$this->assertEquals('Bob', $originalUser->name);
$this->assertEquals('Fred', $syncedUser->name);

As the note above states, the $user object has been changed, without any direct modification.

For completeness, the full code is below:

SyncUserDataTest

class SyncUserDataTest extends TestCase
{
    use UserDataMapper;

    protected $syncUserData;

    public function setUp()
    {
        parent::setUp();

        $this->syncUserData = new App\Helpers\SyncUserData();
    }
    public function test_it_returns_the_unmodified_user()
    {
        $user = factory(App\User::class, 'withCompany')
            ->make([
                'name' => 'Bob',
                'member_id' => 1
            ]);
        $userData = factory(App\UserData::class)
            // And the data is the same
            ->make($this->mapUserToUserData($user) + ['member_id' => 1]);

        $userData->m_field_id_4 = 'Fred';

        $this->syncUserData->sync($user, $userData);

       // dd($user->name);

        $originalUser = $this->syncUserData->getOriginalUser();
        $syncedUser = $this->syncUserData->getSyncedUser();

        $this->assertEquals('Bob', $originalUser->name);
        $this->assertEquals('Fred', $syncedUser->name);
    }

}

SyncUserData

<?php

namespace App\Helpers;

class SyncUserData
{
    /**
     * Has any data changed.
     *
     * @var bool
     */
    private $hasChanged = false;

    /**
     * The original User object without modifications.
     *
     * @var \App\User
     */
    private $originalUser;

    /**
     * The updated User object that has had data synced from the UserData object.
     *
     * @var \App\User
     */
    private $syncedUser;

    /**
     * Sync data from UserData with a User object.
     *
     * @param   \App\User $user
     * @param   \App\UserData $against
     * @return  $this
     */
    public function sync($user, $against)
    {
        $this->setOriginalUser($user);

        $updatedUser = $this->applyChangesToUser($user, $against);

        $this->setSyncedUser($updatedUser);

        return $this;
    }

    /**
     * Check if two values are the same.
     *
     * @param string $check
     * @param string $against
     * @return bool
     */
    public function hasChanged($check, $against)
    {
        if ($check != $against) {
            $this->hasChanged = true;

            return true;
        }

        return false;
    }

    /**
     * @param \App\User $user
     * @return $this
     */
    public function setSyncedUser($user)
    {
        $this->syncedUser = $user;
        return $this;
    }

    /**
     * @param \App\User $user
     * @return $this
     */
    public function setOriginalUser($user)
    {
        $this->originalUser = $user;
        return $this;
    }


    /**
     * Get the updated User object with data synced from the UserData object.
     *
     * @access public
     * @return mixed
     */
    public function getSyncedUser()
    {
        return $this->syncedUser;
    }

    /**
     * Get the original User object.
     *
     * @access public
     * @return mixed
     */
    public function getOriginalUser()
    {
        return $this->originalUser;
    }

    /**
     * Was any data different between the User and UserData objects.
     *
     * @access public
     * @return bool
     */
    public function wasDataChanged()
    {
        return $this->hasChanged;
    }

    /**
     * @param $user
     * @param $against
     */
    private function applyChangesToUser($user, $against)
    {
        if ($this->hasChanged($user->name, $against->m_field_id_4)) {
            $user->name = $against->m_field_id_4;
        }

        if ($this->hasChanged($user->surname, $against->m_field_id_5)) {
            $user->surname = $against->m_field_id_5;
        }

        if ($this->hasChanged($user->legalEntity->name, $against->m_field_id_6)) {
            // TODO This should fire an event to inform the managers that the user has changed company
        }

        if ($this->hasChanged($user->title, $against->m_field_id_8)) {
            $user->title = $against->m_field_id_8;
        }

        if ($this->hasChanged($user->town_city, $against->m_field_id_9)) {
            $user->town_city = $against->m_field_id_9;
        }

        if ($this->hasChanged($user->mobile, $against->m_field_id_10)) {
            $user->mobile = $against->m_field_id_10;
        }

        if ($this->hasChanged($user->position, $against->m_field_id_11)) {
            $user->position = $against->m_field_id_11;
        }

        if ($this->hasChanged($user->department, $against->m_field_id_13)) {
            $user->department = $against->m_field_id_13;
        }

        if ($this->hasChanged($user->county, $against->m_field_id_17)) {
            $user->county = $against->m_field_id_17;
        }

        if ($this->hasChanged($user->country, $against->m_field_id_18)) {
            $user->country = $against->m_field_id_18;
        }

        if ($this->hasChanged($user->telephone, $against->m_field_id_19)) {
            $user->telephone = $against->m_field_id_19;
        }

        if ($this->hasChanged($user->street, $against->m_field_id_20)) {
            $user->street = $against->m_field_id_20;
        }

        if ($this->hasChanged($user->postcode, $against->m_field_id_22)) {
            $user->postcode = $against->m_field_id_22;
        }

        return $user;
    }

}
0 likes
1 reply
DMA's avatar
DMA
OP
Best Answer
Level 2

For anyone who might come across this, it was caused by the nature of PHP passing objects by reference - which wasn't the desired behaviour in this case, so clone $user solves it.

Please or to participate in this conversation.