Skip to content

feat(bdd-db-state-mgmt): Create a helper function and a hook to capture database state - BED-9631 - #134

Merged
ykaiboussiSO merged 1 commit into
mainfrom
BED-9631
Sep 14, 2026
Merged

ykaiboussiSO merged 1 commit into
mainfrom
BED-9631

Conversation

@ykaiboussiSO

@ykaiboussiSO ykaiboussiSO commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

Tasks:

  • Create a helper function that captures the node and relationship counts
  • Sort the expected and actual rows before the assertion
  • Update the step definition to align with the TCK feature files
  • Add unit tests

Resolves: BED-9631

Type of Change

  • Chore (a change that does not modify the application functionality)
  • Bug fix (a change that fixes an issue)
  • New feature / enhancement (a change that adds new functionality)
  • Refactor (no behaviour change)
  • Test coverage
  • Build / CI / tooling
  • Documentation

Testing

  • Unit tests added / updated
  • Integration tests added / updated
  • Full test suite run (make test_all with CONNECTION_STRING set)

Screenshots (if appropriate):

Driver Impact

  • PostgreSQL driver (drivers/pg)
  • Neo4j driver (drivers/neo4j)

Checklist

  • Code is formatted
  • All existing tests pass
  • go.mod / go.sum are up to date if dependencies changed

Summary by CodeRabbit

  • Bug Fixes

    • BDD query result validation now supports any result order and multiple columns.
    • Added verification for relationship query results.
    • Improved graph value formatting, including nodes with empty properties.
    • Scenarios now reset database state before execution and detect unintended graph changes.
  • Tests

    • Expanded coverage for relationship matching, result formatting, unsupported values, column preservation, and side-effect detection.

@ykaiboussiSO ykaiboussiSO self-assigned this Sep 10, 2026
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 0e4c5fc2-69fb-4c0a-aba4-cfee9b801ccd

📥 Commits

Reviewing files that changed from the base of the PR and between 0ea9646 and 5861e31.

📒 Files selected for processing (4)
  • bdd/features/matching.feature
  • bdd/steps_test.go
  • bdd/utils.go
  • bdd/utils_test.go

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


Walkthrough

The BDD harness now matches query rows without regard to order, preserves multiple columns, formats relationships, resets state between scenarios, and detects node or relationship count changes after queries.

Changes

Graph query matching

Layer / File(s) Summary
Scenario lifecycle and step wiring
bdd/utils.go, bdd/steps_test.go, bdd/utils_test.go
Scenarios reset graph and recorded query state before execution. Step wiring uses order-independent result checks and registers the no-side-effects assertion.
Query result capture and formatting
bdd/utils.go, bdd/utils_test.go, bdd/features/matching.feature
Query results now use multi-column rows with order-independent comparison. Node and relationship values receive deterministic formatting, including normalized properties and unsupported-value errors.
Graph state and relationship validation
bdd/utils.go, bdd/utils_test.go, bdd/features/matching.feature
The context captures node and relationship counts before and after queries. Tests and feature scenarios validate relationship matching and graph state preservation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 5861e

This updates BDD query matching and graph-state checks without an identified merge-blocking production risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: adding a BDD database-state helper and hook for node and relationship counts. It is specific and related to the pull request objective.
Description check ✅ Passed The description follows the required template, explains the changes, references BED-9631, identifies the change type, and records unit test updates. Some checklist items remain unchecked, but the desc…
Full details: Docstring Coverage

Explanation

Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-9631

A rabbit sorts rows left and right,
Keeps every column tucked in tight.
Nodes and links wear labels bright,
Fresh scenes reset before the bite.
No graph changes hide from sight.

Comment @coderabbitai help to get the list of available commands.

Comment thread bdd/utils.go
return builder.String()
}

// noSideEffects verifies that the query did not change graph node or relationship counts.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: It looks like TCK checks properties and labels in addition to nodes and relationships as described here. Is there some sort of limitation we are running into for fully implementing no side effects or is this something we plan to add with a different changeset?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@urangel It’s a placeholder for now. We plan to add a changeset with scenarios for creating relationships that capture the side effects.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay sounds good as long as we are tracking that somewhere. Otherwise some tck tests might give false positives.

@ykaiboussiSO
ykaiboussiSO merged commit 8a5f8f3 into main Sep 14, 2026
13 checks passed
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