Skip to content

Conversation

@xenco
Copy link

@xenco xenco commented Dec 17, 2017

See the added part in README for further explanation.

@timgws
Copy link
Owner

timgws commented Dec 18, 2017

Hi xenco!

Thanks for your first commit to the project!

To help ensure that there is high quality code inside this project, you will note that we have automated testing. This is provided by a service called Travis.

When a new pull request is made (or changes are made to a branch with a pull request), Travis will automatically try running the code base in a few different configurations.

This automatic test has failed.

2) timgws\test\JoinSupportingQueryBuilderParserTest::testDateBetween
timgws\QBParseException: Field (dollar_amount) is in no list

/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QBPFunctions.php:276
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:251
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/JoinSupportingQueryBuilderParser.php:70
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:80
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:60
/home/travis/build/timgws/QueryBuilderParser/tests/JoinSupportingQueryBuilderParserTest.php:299

It appears that some of the logic for comparing fields in the database may be broken, as most of the tests have failed.

To run these tests manually on your computer when developing, run these commands inside the project folder:

composer install --prefer-source --no-interaction --dev
vendor/bin/phpunit --testsuite MySQL --coverage-clover ./tests/logs/clover.xml

Also, feel free to ask for any help if you need it!

@xenco
Copy link
Author

xenco commented Dec 18, 2017

It doesn't break the default functionality anymore.
But I'm not sure how to test the added features. I can't just check the produced (prepared) queries, because they need the bindings, which themselves need some test data sets.
How would I create a test database with migration and seeding for sqlite:memory in isolation? Any help is appreciated.

@timgws
Copy link
Owner

timgws commented Jan 15, 2018

I can't just check the produced (prepared) queries, because they need the bindings

You actually should be able to do that. I don't think that doing a full test is really all that necessary.

We only need to make sure that we correctly generate queries that we expect. We don't need to do an end to end test, as that would be the responsibility of the project that is adding this library as a dependency.

Here is an example test that I have written: https://github.com/timgws/QueryBuilderParser/blob/master/tests/QueryBuilderParserTest.php#L272-L286

@timgws
Copy link
Owner

timgws commented Jan 15, 2018

That being said, here is an instance where the project does test bindings:

/**
* Test for #21 (Cast datetimes and add 'not between' operator)
*/
public function testDateNotBetween()
{
$incoming = '{ "condition": "AND", "rules": [ { "id": "needed_by_date", "field": "needed_by_date", "type": "date", "input": "text", "operator": "not_between", "value": [ "10/22/2017", "10/28/2017" ] } ], "not": false, "valid": true }';
$builder = $this->createQueryBuilder();
$qb = $this->getParserUnderTest();
$qb->parse($incoming, $builder);
$this->assertEquals('select * where `needed_by_date` not between ? and ?', $builder->toSql());
$bindings = $builder->getBindings();
$this->assertCount(2, $bindings);
$this->assertInstanceOf("Carbon\\Carbon", $bindings[0]);
$this->assertInstanceOf("Carbon\\Carbon", $bindings[1]);
$this->assertEquals(2017, $bindings[0]->year);
$this->assertEquals(22, $bindings[0]->day);
$this->assertEquals(28, $bindings[1]->day);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants