Sequence integer query test

[fogelito]

Summary by CodeRabbit

• Tests
  • Use strict identity checks when validating sequence values to prevent type-coercion issues.
  • Added an adapter-specific test verifying handling of very large integer sequence values.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8fadbc79-7ef3-4c53-9048-b32e7435b40f

📥 Commits

Reviewing files that changed from the base of the PR and between 0daa124 and 08c2e49.

📒 Files selected for processing (1)

• tests/e2e/Adapter/Scopes/DocumentTests.php

---

📝 Walkthrough

Walkthrough

The test testBigintSequence() in tests/e2e/Adapter/Scopes/DocumentTests.php was updated to use strict identity assertions and to add an adapter-type branch: when the adapter ID attribute is Database::VAR_INTEGER it performs an integer-based findOne lookup and asserts the large sequence value and retrieved sequence strictly.

Changes

Cohort / File(s)	Summary
Test: bigint sequence assertions & adapter branch tests/e2e/Adapter/Scopes/DocumentTests.php	Replaced loose equality checks with assertSame for sequence values across createDocument, getDocument, and findOne. Added branch for adapters with Database::VAR_INTEGER ID attribute to run an integer findOne lookup, assert the sequence equals 5000000000000000, and assert the retrieved document sequence strictly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Possibly related PRs

• Primary bigint #609: Modifies the same testBigintSequence test and addresses bigint primary/sequence handling; closely related.
• Refactor getSequences #618: Refactors adapter sequence handling affecting how sequences are populated and returned; relevant to these assertions.
• Cahnge getSequence default for null #650: Changes in document sequence representation and test assertions intersect with this test update.

Suggested reviewers

• abnegate
• ArnabChatterjee20k

Poem

🐰 I hopped through tests with numbers so grand,

Five quadrillion held gently in hand.
I check with strict eyes, no loose disguise,
Adapters and integers meet my surprise.
Hop, assert, repeat—QA's carrot prize. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Sequence integer query test' directly and specifically describes the main change: adding integer-specific query logic to the sequence test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch query-integer-sequence

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR attempts to extend testBigintSequence with a test case that verifies SQL adapters can accept a native PHP integer (rather than a string) in Query::equal('$sequence', [$sequence]). The intent is valid and fills a real coverage gap, but the added block was clearly not cleaned up before submission.

• var_dump() calls (lines 100, 103) were left in and will pollute test output.
• $this->assertTrue(false) (line 104) is an unconditional failing assertion — every SQL-adapter CI run will fail at this line regardless of actual database behavior.
• $this->assertTrue($sequence === 5_000_000_000_000_000) (line 101) only validates the value of a local PHP variable that was already set two lines earlier; it adds no meaningful database coverage.
• The block needs to be replaced with a proper assertion on the returned documents (e.g. assertCount and assertEquals on the sequence value) and the debug scaffolding removed before merging.

Confidence Score: 1/5

• Not safe to merge — the added code contains a hard-coded assertTrue(false) that will unconditionally fail the test suite on all SQL adapters.
• The sole change introduces debug artifacts (var_dump, assertTrue(false)) that break CI. The feature intent is reasonable but the implementation is clearly unfinished work-in-progress.
• tests/e2e/Adapter/Scopes/DocumentTests.php — the new test block must be rewritten before merging.

Important Files Changed

Filename	Overview
tests/e2e/Adapter/Scopes/DocumentTests.php	Adds a conditional test block for integer $sequence queries, but it contains two var_dump() debug calls, a hard-coded assertTrue(false) that guarantees test failure, and an assertion that only checks a local variable's value rather than the query result.

_{Reviews (1): Last reviewed commit: "$sequence int" | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/e2e/Adapter/Scopes/DocumentTests.php`:
- Around line 99-105: Remove the forced failure and replace the debug dumps with
real assertions: when $database->getAdapter()->getIdAttributeType() ==
Database::VAR_INTEGER (Database::VAR_INTEGER), assert that $sequence equals the
expected bigint literal, perform the lookup via $database->find(__FUNCTION__,
[Query::equal('$sequence', [$sequence])]) and assert the returned $document is
found (not null/false) and that its $sequence field matches $sequence; delete
the var_dump calls and the assertTrue(false) so the test verifies the integer
lookup rather than failing unconditionally.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f12fb248-f850-45fa-b229-056cc1d8d5ec

📥 Commits

Reviewing files that changed from the base of the PR and between cff2b6e and 0daa124.

📒 Files selected for processing (1)

• tests/e2e/Adapter/Scopes/DocumentTests.php