Remove global regex limit due to permissions error #784
Conversation
📝 WalkthroughWalkthroughRemoved direct PDO execution that set MySQL's global Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-03T02:04:17.803ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/Adapter/Scopes/DocumentTests.php (1)
7243-7465: Remove the orphan PHPDoc + commented-out method block; use the file's existing skip pattern instead.The orphan PHPDoc followed by a fully commented-out method violates PSR-12's statement_indentation rule (the commented block has inconsistent indentation). However, the proposed fix should align with the skip pattern already used throughout this file:
expectNotToPerformAssertions()+ early return, notmarkTestSkipped().Proposed fix (using existing codebase pattern)
/** * Test ReDoS (Regular Expression Denial of Service) with timeout protection * This test verifies that ReDoS patterns either timeout properly or complete quickly, * preventing denial of service attacks. */ -// public function testRegexRedos(): void -// { -// /** `@var` Database $database */ -// $database = static::getDatabase(); -// ... -// } + public function testRegexRedos(): void + { + // TODO: Adapt to per-query timeout strategy and skip (not fail) when environment cannot enforce timeout. + $this->expectNotToPerformAssertions(); + return; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/e2e/Adapter/Scopes/DocumentTests.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
Applied to files:
tests/e2e/Adapter/Scopes/DocumentTests.php
🪛 GitHub Actions: Linter
tests/e2e/Adapter/Scopes/DocumentTests.php
[error] 1-1: PSR-12: statement_indentation violation detected by Pint. Run 'vendor/bin/pint' to fix formatting. Command that failed: 'php -d memory_limit=2G ./vendor/bin/pint --test'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (MongoDB)
- GitHub Check: Adapter Tests (Schemaless/MongoDB)
- GitHub Check: Adapter Tests (SharedTables/MongoDB)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Unit Test
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/Adapter/Scopes/DocumentTests.php (1)
7243-7465: Replace the commented-out test with an explicit skip and remove the dead code.The
testRegexRedos()test is currently disabled via ~200 lines of comments, which hides it from CI and leaves dead code. UsemarkTestSkipped()instead, with a reason (e.g., "pending stability across adapters"). This improves CI signal and allows proper git history tracking.Note: ReDoS coverage is now minimal—only a basic
< 1.0sassertion on pattern(a+)+$intestFindRegex()(line 7005–7015). If the commented test is being re-enabled with the new per-querymax_execution_timeapproach, consider doing so explicitly rather than leaving it hidden.Suggested change
/** * Test ReDoS (Regular Expression Denial of Service) with timeout protection * This test verifies that ReDoS patterns either timeout properly or complete quickly, * preventing denial of service attacks. */ -// public function testRegexRedos(): void -// { -// ... -// } + public function testRegexRedos(): void + { + $this->markTestSkipped('ReDoS timeout validation temporarily skipped pending per-query timeout stability.'); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/e2e/Adapter/Scopes/DocumentTests.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
Applied to files:
tests/e2e/Adapter/Scopes/DocumentTests.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (MariaDB)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.