Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to mocking database trans_status() in controller #384

Open
anggadarkprince opened this issue Sep 14, 2021 · 25 comments
Open

How to mocking database trans_status() in controller #384

anggadarkprince opened this issue Sep 14, 2021 · 25 comments
Labels

Comments

@anggadarkprince
Copy link

anggadarkprince commented Sep 14, 2021

In my controller I'm using database transaction, I want to simulate when transaction is failed

$this->db->trans_start();
   ... query insert / updates
$this->db->trans_complete();

if ($this->db->trans_status()) {
   // redirect...
} else {
   // I want to check this else
}

in my test, I also implement transaction rollback

$this->db = $this->CI->load->database('testing', TRUE);
$this->db->trans_begin();
$DB = $this->db;
$this->request->setCallablePreConstructor(
   function () use ($DB) {
      load_class_instance('db', $DB);
   }
 );

rollback on tearDown()
$this->db->trans_rollback();

how I can achieve this? I try mockBuilder not working

@kenjis
Copy link
Owner

kenjis commented Sep 15, 2021

If you believe CodeIgniter's database library works fine, I recommend you use mock.

See how to mock $this->db: https://github.com/kenjis/ci-app-for-ci-phpunit-test/blob/v3.0.0/application/tests/models/Category_model_mocking_db_test.php

Mock the all methods you use in your controller, and inject the mock object into the controller $db.

@anggadarkprince
Copy link
Author

I want to keep the DML (insert, edit delete) method, only mocking result of $this->db->trans_status(), when I try to follow the example, insert, edit, delete error, but I'm managed to use MonkeyPatch::patchMethod('CI_DB_driver', ['trans_status' => false]); even when try to check the data it's really inserted already

@kenjis
Copy link
Owner

kenjis commented Sep 17, 2021

I don't recommend you use monkey patch.
It's dirty solution for legacy code.

What do you want to test? CodeIgniter code? DBMS? or your app code?

When you use the real db connection, you test CodeIgniter code and DBMS and your code.

I'm managed to use MonkeyPatch::patchMethod('CI_DB_driver', ['trans_status' => false]); even when try to check the data it's really inserted already

$this->db->trans_complete() runs $this->trans_commit() or $this->trans_rollback().
If $this->trans_commit() runs, the data are saved in the database.

@anggadarkprince
Copy link
Author

anggadarkprince commented Sep 23, 2021

I want to test how my application can react (catch) against database failure, in my controller I do many database operation such as inserts and updates using transaction. I want to make sure that if the transaction fails, it will through the else block, which is return to the form and showing the error message to user, that some error occurred, then they can submit the form again.
So I decide to mock the $this->db->trans_status() result, I want to make it return false on purpose, I'm ignoring the insert/update process (It's ok if the data inserted via $this->trans_commit() inside $this->db->trans_complete(), I just want to test the else block. I try to use getMockBuilder() and set method() to mock only the trans_status() function but, another function return null

@kenjis
Copy link
Owner

kenjis commented Sep 24, 2021

Call setMethods() with the method names you want to mock.

$mock = $this->getMockBuilder(stdClass::class)
                     ->setMethods(['set'])
                     ->getMock();

See https://phpunit.readthedocs.io/en/8.5/test-doubles.html

If you use PHPUnit 9, use onlyMethods() instead.
See https://phpunit.readthedocs.io/en/9.5/test-doubles.html

@kenjis
Copy link
Owner

kenjis commented Sep 24, 2021

You could use $this->getDouble() short cut method that ci-phpunit-test provides.

See https://github.com/kenjis/ci-phpunit-test/blob/3.x/docs/FunctionAndClassReference.md#testcasegetdoubleclassname-params-constructor_params--false

@anggadarkprince
Copy link
Author

anggadarkprince commented Oct 2, 2021

I replace the code bellow:

//MonkeyPatch::patchMethod('CI_DB_driver', ['trans_status' => false]);
$this->request->addCallable(function ($CI) {
	$db = $this->getMockBuilder('CI_DB_driver')->disableOriginalConstructor()->getMock();
	$db->method('trans_status')->willReturn(false);
	$CI->db = $db;
});

and result error Error: Call to undefined method Mock_CI_DB_driver_11f0bbca::select()

@kenjis
Copy link
Owner

kenjis commented Oct 2, 2021

CI_DB_driver does not have select().

Mock the real class you actually use. If you use mysqli driver:

$this->request->addCallable(function ($CI) {
	$db = $this->getMockBuilder('CI_DB_mysqli_driver')->disableOriginalConstructor()->getMock();
	$db->method('trans_status')->willReturn(false);
	$CI->db = $db;
});

@anggadarkprince
Copy link
Author

I already tried before, I'm using mysqli then result another error Error: Call to a member function from() on null

@kenjis
Copy link
Owner

kenjis commented Oct 2, 2021

To what did you call from()? Inspect it, please.
Error messages tells you what is wrong.

@anggadarkprince
Copy link
Author

anggadarkprince commented Oct 2, 2021

I don't call any database operation from test, error come from the controller which I called from $this->request because the I mock the $db. The message lack of information, which line produce the error or where the null $db is. The stack trace pointing wrong line number of code, when I checked the line it is pointing the comment or the code that I never test.

Ah i know where the line of code pointing not our source code but in /application/tests/_ci_phpunit_test/temp/cache/src
and error when I select data from db in controller, this is what I meant in first question above, why $db in model become null?

@kenjis
Copy link
Owner

kenjis commented Oct 2, 2021

The stack trace pointing wrong line number of code, when I checked the line it is pointing the comment or the code that I never test.

It is because you use Monkey Patching.
See https://github.com/kenjis/ci-phpunit-test/blob/3.x/application/tests/_ci_phpunit_test/ChangeLog.md#changed-2

@kenjis
Copy link
Owner

kenjis commented Oct 2, 2021

this is what I meant in first question above, why $db in model become null?

$db in model? What do you mean? Not in controller? And not $db but $this->db?
I don't have your code, so I don't know why.

Why don't you run step execution and debug?

@anggadarkprince
Copy link
Author

anggadarkprince commented Oct 4, 2021

the controller call method from model that selecting data from database

public function inactive($id)
{
     // this line is error in test, it says error call select() on null
     // getById($id) contains $this->db->from('trainings')->select('trainings.*')->where(['id' => $id])->get()->row_array()
    $training = $this->training->getById($id);

    $this->db->trans_start();

    $this->training->update([
	'status' => TrainingModel::STATUS_INACTIVE,
    ], $id);

    $this->statusHistory->create([
	'type' => StatusHistoryModel::TYPE_TRAINING,
	'id_reference' => $id,
	'status' => TrainingModel::STATUS_INACTIVE,
	'description' => $this->input->post('message'),
    ]);

    $this->db->trans_complete();

    // I want to mock the trans_status() result to false, so it will go the else block
    if ($this->db->trans_status()) {
	flash('warning', "Training {$training['curriculum_title']} for employee {$training['employee_name']} is successfully deactivated");
    } else {
        // I want to check this line
	flash('danger', "Deactivate training failed, try again or contact administrator");
    }
    redirect('training/class');
}

@kenjis
Copy link
Owner

kenjis commented Oct 4, 2021

Why don't you mock $this->training, too?

Ah, you probably forget calling setMethods()/onlyMethods()?
#384 (comment)

@kenjis
Copy link
Owner

kenjis commented Oct 4, 2021

This test passed.

<?php

/**
 * @property CI_DB_pdo_sqlite_driver $db
 */
class Db_trans extends CI_Controller
{
	public function __construct()
	{
		parent::__construct();

		$this->load->database();
	}

	public function index()
	{
		$this->db->trans_start();

		$this->db->trans_complete();

		if ($this->db->trans_status()) {
			echo 'Okay';
		} else {
			echo 'Error';
		}
	}
}
<?php

class Db_trans_test extends TestCase
{
	public function test_index()
	{
		$this->request->addCallable(function ($CI) {
			$db = $this->getMockBuilder('CI_DB_pdo_sqlite_driver')
					->disableOriginalConstructor()
					->setMethods([
						'trans_status',
						'trans_start',
					])
					->getMock();
			$db->method('trans_status')->willReturn(false);
			$db->method('trans_start')->willReturn(true);
			$CI->db = $db;
		});

		$output = $this->request('GET', 'db_trans');

		$this->assertSame('Error', $output);
	}
}

@anggadarkprince
Copy link
Author

anggadarkprince commented Oct 5, 2021

setMethods() is deprecated but I already add setMethods()/onlyMethods() and the error still same. In my test using database transaction to rollback data after request, is this caused the error?

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

    if ($this->databaseTransactions) {
	$this->resetInstance();
	$this->db = $this->CI->load->database('testing', TRUE);
	$this->db->trans_begin();
	$DB = $this->db;

	$this->request->setCallablePreConstructor(
		function () use ($DB) {
			// Inject db object
			load_class_instance('db', $DB);
		}
	);
    }
}

public function tearDown(): void
{
    if ($this->databaseTransactions) {
	if (is_object($this->db->conn_id) || is_resource($this->db->conn_id)) {
		$this->db->trans_rollback();
		$this->enableTransaction(true);
	}
    }

    parent::tearDown();
}

@kenjis
Copy link
Owner

kenjis commented Oct 5, 2021

You inject the real db object in your setup() and use it to rollback in tearDown().
It means you use the real db connection, and read/write db, and after testing rollback.

It is recommended to write tests that either access the DB or use mocks to not access the DB at all.

It is possible to mock only a part of db access, but I don't think it would make much sense as it would only increase the complexity.

@kenjis
Copy link
Owner

kenjis commented Oct 5, 2021

In my test using database transaction to rollback data after request, is this caused the error?

Maybe no. The db object in the setUp() and the mock db object are two different objects.

@kenjis
Copy link
Owner

kenjis commented Oct 5, 2021

setMethods() is deprecated but I already add setMethods()/onlyMethods() and the error still same.

Use onlyMethods().

Any way, all you to do is to fix the db mock not to cause errors.
https://phpunit.readthedocs.io/en/9.5/test-doubles.html#mock-objects

If you get Error: Call to a member function xxx() on null, mock method so that null is not returned in the db.

@anggadarkprince
Copy link
Author

anggadarkprince commented Oct 5, 2021

I use the example of your project ci-app-for-ci-phpunit-test, in other framework that I used to, they have capability to rollback out of the box, no need manually setup. In this one I just copy from the example code, is it wrong with that? I want to run integration testing so I need to write in real database.

Any way, all you to do is to fix the db mock not to cause errors.

Why I need to mock others method in DB object? when I try to mock model it's fine to mock single method only, and other method not affected by that.

I'm sorry if I got confuse and frustrated to resolve this problem, I'm beginner at Software Testing, in many case I just test simple functionality of application such as validate result of API or something similar

@kenjis
Copy link
Owner

kenjis commented Oct 5, 2021

In this one I just copy from the example code, is it wrong with that?

The test code is not wrong, because all the tests pass in the repository.
But it is only correct in the context of the repository. If you use it in another context (your project),
something might be broken.

Why I need to mock others method in DB object?

Because the mock you create can't access the database at all.

I figured out what you exactly want to do.
You need a partial mock which can access the database.

        $db['test'] = array(
            'dsn'	=> '',
            'hostname' => 'localhost',
            'username' => 'username',
            'password' => 'password',
            'database' => 'codeigniter',
            'dbdriver' => 'mysqli',
            'dbprefix' => '',
            'pconnect' => FALSE,
            'db_debug' => TRUE,
            'cache_on' => FALSE,
            'cachedir' => '',
            'char_set' => 'utf8',
            'dbcollat' => 'utf8_general_ci',
            'swap_pre' => '',
            'encrypt' => FALSE,
            'compress' => FALSE,
            'stricton' => FALSE,
            'failover' => array(),
            'save_queries' => TRUE
        );
        $params = $db['test'];
        $this->request->addCallable(function ($CI) use ($params) {
            $db = $this->getMockBuilder('CI_DB_mysqli_driver')
                ->setConstructorArgs([$params])
                ->setMethods([
                                 'trans_status',
                             ])
                ->getMock();
            $db->method('trans_status')->willReturn(false);
            $CI->db = $db;
        });

If you get header error, add @runInSeparateProcess in the test method.

        /**
	 * @runInSeparateProcess
	 */
	public function test_index()

@kenjis
Copy link
Owner

kenjis commented Oct 5, 2021

You can write like this with getDouble():

        $db['test'] = array(
            'dsn'	=> '',
            'hostname' => 'localhost',
            'username' => 'username',
            'password' => 'password',
            'database' => 'codeigniter',
            'dbdriver' => 'mysqli',
            'dbprefix' => '',
            'pconnect' => FALSE,
            'db_debug' => TRUE,
            'cache_on' => FALSE,
            'cachedir' => '',
            'char_set' => 'utf8',
            'dbcollat' => 'utf8_general_ci',
            'swap_pre' => '',
            'encrypt' => FALSE,
            'compress' => FALSE,
            'stricton' => FALSE,
            'failover' => array(),
            'save_queries' => TRUE
        );
        $params = $db['test'];
        $this->request->addCallable(function ($CI) use ($params) {
            $db = $this->getDouble(
                'CI_DB_mysqli_driver',
                [
                    'trans_status' => false,
                ],
                [$params]
            );

            $CI->db = $db;
        });

@anggadarkprince
Copy link
Author

You need a partial mock which can access the database.

exactly, I just want to mock the trans_status() to return false, any functionality to database still same

Ah I see, the mock object depends on configuration which passed from the constructor to making connection,
but I think it should call initialized() method, because the code still caused error Error: Call to a member function real_escape_string() on bool

$params = $db['test'];
$this->request->addCallable(function ($CI) use ($params) {
	$db = $this->getMockBuilder('CI_DB_mysqli_driver')
		->setConstructorArgs([$params])
		->onlyMethods([
			'trans_status',
		])
		->getMock();
	$db->method('trans_status')->willReturn(false);
	$db->initialize(); // I add this part
	$CI->db = $db;
});

but test run longer and error again Lock wait timeout exceeded; try restarting transaction

@anggadarkprince
Copy link
Author

anggadarkprince commented Apr 27, 2022

I found the solution but not totally perfect, due to mocked db that passed into the request is different object with db in test case that used for rollback the data state, and controller will not see changes of the test (created data or anything), so we need can use existing data to when run the test that available in both connection session.

// $db in test case (used for rollback) is different with $db we mocked then
// we need use same data that exist before the test is running
$existingUser = $this->db->get_where('prv_users', ['id' => 1])->row_array();
$this->loginAs($existingUser, PERMISSION_SETTING_EDIT);

// this data will not available in the request
// $this->hasInDatabase('logs', []);

$this->request->addCallable(function ($CI) {
	include(APPPATH . 'config/testing/database.php');
	$params = $db['testing'] ?? [];
	$db = $this->getMockBuilder('CI_DB_mysqli_driver')
		->setConstructorArgs([$params])
		->onlyMethods([
			'trans_status',
			'trans_complete',
		])
		->getMock();
	$db->method('trans_status')->willReturn(false);
	$db->method('trans_complete')->willReturnCallback(function () use ($db) {
		$db->trans_rollback(); // rollback $db in the controller
	});
	$db->initialize();
        
        // $db->insert('prv_users', []) // you can add transaction here that rollback in controller

	$CI->db = $db; // all data changes of this test will not available in controller)
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants