chore: test functions against a real PostgreSQL server #379
Conversation
WalkthroughMultiple new integration test classes were introduced to cover various custom Doctrine ORM DQL functions for JSONB operations, including insertion, aggregation, path queries, pretty-printing, key extraction, set operations, and null stripping. Existing tests were updated for improved coverage and accuracy, with some test methods renamed, expanded, or removed. The JSON test data was extended with additional JSON objects for broader test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant TestClass as Integration Test Class
participant Doctrine as Doctrine ORM
participant DB as Database
TestClass->>Doctrine: Register custom DQL JSONB function
TestClass->>Doctrine: Prepare DQL query with JSONB function
TestClass->>DB: Execute query
DB-->>Doctrine: Return query result (JSONB data)
Doctrine-->>TestClass: Return result
TestClass->>TestClass: Decode JSON, assert structure/values
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPrettyTest.php (1)
1-26: Well-structured test for the JsonbPretty function.This new test class follows the established pattern for testing JSON functions. It properly registers the function, constructs an appropriate test query, and verifies the basic functionality by checking the return type.
Consider enhancing this test by also verifying the format of the prettified JSON, not just that it returns a string:
$result = $this->executeDqlQuery($dql); $this->assertIsString($result[0]['result']); +$this->assertStringContainsString('\n', $result[0]['result'], 'Pretty JSON should contain newlines'); +$this->assertStringContainsString(' ', $result[0]['result'], 'Pretty JSON should contain indentation');tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectKeysTest.php (1)
1-26: Well-structured test class for JsonbObjectKeys function.This test follows the established pattern for JSON function tests and verifies the basic functionality. However, the assertion only checks that the result is a string but doesn't validate the actual keys extracted.
Consider enhancing the test with more specific assertions:
$result = $this->executeDqlQuery($dql); $this->assertIsString($result[0]['result']); +// Decode the result string which should be a JSON array of keys +$keys = json_decode($result[0]['result'], true); +$this->assertIsArray($keys); +$this->assertContains('name', $keys); +$this->assertContains('age', $keys); +$this->assertContains('tags', $keys); +$this->assertContains('address', $keys);This would provide stronger verification that the function is returning the expected object keys.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbStripNullsTest.php (2)
27-36: String assertions could be more robustWhile the current approach works, string comparison with
assertStringNotContainsStringto verify null values are removed is potentially brittle, as it depends on specific JSON formatting.Consider decoding the JSON string first and then asserting on the structure:
- $this->assertStringNotContainsString('"age": null', $result[0]['result']); - $this->assertStringNotContainsString('"zip": null', $result[0]['result']); + $decoded = \json_decode($result[0]['result'], true); + $this->assertIsArray($decoded); + $this->assertArrayHasKey('name', $decoded); + $this->assertArrayNotHasKey('age', $decoded) || $this->assertNotNull($decoded['age']); + $this->assertArrayNotHasKey('zip', $decoded['address']) || $this->assertNotNull($decoded['address']['zip']);
18-25: Consider adding assertions for the first test caseThe first test only asserts that the result is a string but doesn't verify the actual content.
Consider adding assertions to verify the content of the result, similar to the second test case:
$this->assertIsString($result[0]['result']); +$decoded = \json_decode($result[0]['result'], true); +$this->assertIsArray($decoded); +$this->assertSame('John', $decoded['name']);tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (1)
80-80: Minor inconsistency in assertion styleThere's an inconsistency in assertion style between
self::assertArrayNotHasKeyhere and$this->assert*used in the rest of the tests. For consistency, consider using the same style throughout.- self::assertArrayNotHasKey('nonexistent', $decoded); + $this->assertArrayNotHasKey('nonexistent', $decoded);tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetLaxTest.php (2)
80-80: Minor inconsistency in assertion styleThere's an inconsistency in assertion style between
self::assertArrayNotHasKeyhere and$this->assert*used in the rest of the tests. For consistency, consider using the same style throughout.- self::assertArrayNotHasKey('invalid', $decoded); + $this->assertArrayNotHasKey('invalid', $decoded);
68-81: Consider adding a comment explaining the difference between JSONB_SET and JSONB_SET_LAXIt would be helpful to add a brief comment explaining that the key difference between JSONB_SET and JSONB_SET_LAX is how they handle invalid paths - the LAX variant doesn't throw errors but silently ignores invalid paths.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (1)
55-63: Consider enhancing exception assertionsThe test correctly expects an exception when inserting at an existing path, but it doesn't verify the specific error message or code. This would make the test more robust against changes in exception types while preserving the expected behavior.
$this->expectException(Exception::class); + $this->expectExceptionMessage('path exists'); // Add the expected error message $dql = 'SELECT JSONB_INSERT(t.object1, :path, :value) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsJsons t WHERE t.id = 1';
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonTestCase.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbAggTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectAggTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectKeysTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPrettyTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetLaxTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbStripNullsTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
🧬 Code Graph Analysis (3)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectKeysTest.php (5)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectKeys.php (1)
JsonbObjectKeys(15-22)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonTestCase.php (1)
JsonTestCase(9-56)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbAggTest.php (1)
getStringFunctions(11-14)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (1)
getStringFunctions(11-16)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(236-250)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArray.php (1)
JsonbPathQueryArray(22-55)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonTestCase.php (1)
JsonTestCase(9-56)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(236-250)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirst.php (1)
JsonbPathQueryFirst(23-56)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonTestCase.php (1)
JsonTestCase(9-56)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(236-250)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: sloth
🔇 Additional comments (17)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonTestCase.php (2)
42-44: Good addition of diverse test data structures.The expanded test data includes important edge cases that will help verify PostgreSQL JSONB functions: empty arrays, empty objects, and null values. This provides more comprehensive test coverage.
49-52: Properly updated SQL insert statement.The SQL statement has been correctly updated to insert all five JSON objects, maintaining the pattern of duplicating each object in both columns.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbAggTest.php (1)
34-38: Method renamed and query refined for better specificity.The test method has been appropriately renamed from
test_jsonb_agg_with_all_rowstotest_jsonb_agg_with_multiple_rowsto more accurately reflect its purpose. The query now filters to specific rows rather than selecting all rows, which is a better testing practice as it makes the test more deterministic.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbObjectAggTest.php (1)
1-29: Implementation looks good!The test correctly validates that
JSONB_OBJECT_AGGfunction works as expected, creating a JSON object from key-value pairs and properly returning it as a string that can be decoded back to an array matching the expected structure.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php (4)
18-32: Well-structured test for simple path queryThe test correctly validates extraction of a single value using a JSON path expression.
34-48: Good test for array extractionThis test effectively verifies that the function can extract multiple values from a JSON array.
50-65: Comprehensive test for filtered path queriesThe test thoroughly validates that path filtering with conditions works correctly by checking both the count and the exact content of the returned array.
67-78: Good validation of column reference usageThis test properly verifies that the function works when referencing a database column instead of a literal JSON string.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php (5)
18-29: Well-structured test for simple path queryThe test correctly validates extraction of a single value using a JSON path expression.
31-42: Good test for array first element extractionThis test effectively verifies that the function returns only the first element when multiple matches exist.
44-55: Thorough test for filtered path queriesThe test properly validates that filtering works and returns the correct first matching element.
57-67: Good edge case testing for no matchesThis test correctly verifies the behavior when no elements match the path query, ensuring the function returns null as expected.
69-77: Good validation of column reference usageThis test properly verifies that the function works when referencing a database column instead of a literal JSON string.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (1)
1-82: Good test coverage for the JSONB_SET function!The tests cover all the essential functionality including updating existing values, adding new values, handling nested paths, and testing the
create_missingflag.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php (1)
1-74: Well-structured tests with good coverage for the JSONB_PATH_QUERY function!The tests cover a comprehensive range of JSON path query scenarios including simple queries, array queries, filtered queries, and column reference queries. The assertions are thorough and validate both the result structure and content.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetLaxTest.php (1)
1-82: Good test coverage for the JSONB_SET_LAX function!The tests effectively cover the essential scenarios including updating existing values, adding new values, handling nested paths, and importantly, testing behavior with invalid paths which is a key differentiator for the LAX variant.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (1)
1-76: Good test coverage for the JSONB_INSERT function!The tests cover the essential functionality including inserting new values, handling nested paths, and testing error conditions.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php
Show resolved
Hide resolved
Summary by CodeRabbit