Skip to content

chores: Add GH Workflow Integration Tests#53

Merged
ykaiboussiSO merged 17 commits intomainfrom
PQE-401
Apr 1, 2026
Merged

chores: Add GH Workflow Integration Tests#53
ykaiboussiSO merged 17 commits intomainfrom
PQE-401

Conversation

@ykaiboussiSO
Copy link
Copy Markdown
Contributor

@ykaiboussiSO ykaiboussiSO commented Mar 31, 2026

Description

  • Adds a GitHub Actions integration-test job for pg and neo4j drivers.

Resolves: Closes: PQE-401

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
  • Manual integration tests run (go test -tags manual_integration ./integration/...)

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

  • Tests

    • Added integration tests for the ops package that run against PostgreSQL and Neo4j backends to validate database interactions.
  • Chores

    • Added Docker Compose configuration to run local PostgreSQL and Neo4j services with persisted volumes.
    • Added CI integration-test jobs for PostgreSQL and Neo4j that execute the integration suite against each backend.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds integration testing: a GitHub Actions workflow runs ops package integration tests against Postgres and Neo4j service containers; a new docker-compose file defines those services; and a new integration test suite (ops/ops_test.go, build tag integration) exercises ops package DB operations against both backends.

Changes

Cohort / File(s) Summary
CI Workflow
\.github/workflows/go-test.yml
Added two integration jobs (integration-test-pg, integration-test-neo4j) that start service containers (Postgres 16, Neo4j 4.4.42), wait for health checks, set Driver_CONNECTION_STRING, and run go test -tags integration -v ./ops/....
Local/Compose Services
docker-compose.yml
New compose file defining postgres (postgres:16) and neo4j (neo4j:4.4.42) services, env credentials, host port mappings, and named volumes pg_data / neo4j_data for persistence.
Integration Tests
ops/ops_test.go
New //go:build integration test suite with TestSuite, dbSetup reading Driver_CONNECTION_STRING (Postgres or Neo4j), cleanDB helper, and tests for DeleteRelationships, FetchRelationships, FetchNodeSet, FetchNodes, and FetchNodesByQuery that seed data and assert expected results.

Sequence Diagram(s)

sequenceDiagram
    participant GH as "GitHub Actions Runner"
    participant Repo as "Repository"
    participant Postgres as "Postgres Container"
    participant Neo4j as "Neo4j Container"
    participant Test as "Integration Tests (ops)"
    GH->>Repo: checkout code
    GH->>Postgres: start postgres service (healthcheck)
    GH->>Neo4j: start neo4j service (healthcheck)
    GH->>GH: setup Go, cache modules
    GH->>Test: run `go test -tags integration ./ops/...` with connection string
    Test->>Postgres: open DB connection / run write/read transactions
    Test->>Neo4j: open DB connection / run write/read transactions
    Postgres-->>Test: return query results
    Neo4j-->>Test: return query results
    Test-->>GH: exit status (pass/fail)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I hopped in with a tiny test plan,

spun up Postgres and Neo4j on demand,
seeded some nodes, relationships in tow,
ran transactions gentle and slow,
CI nodded, carrots all aglow. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% 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
Title check ✅ Passed The title 'chores: Add GH Workflow Integration Tests' accurately describes the main changes: adding integration tests and GitHub Actions workflow configuration.
Description check ✅ Passed Pull request description follows the template with all major sections completed, including type of change, testing, driver impact, and relevant issue reference.

✏️ 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 PQE-401

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
.github/workflows/go-test.yml (3)

78-106: Both database services start for each matrix job.

Currently, both Postgres and Neo4j containers start for every matrix entry, even though each test only uses one. This doubles resource usage and startup time.

GitHub Actions doesn't natively support conditional services, but you could work around this by:

  1. Moving to separate jobs instead of a matrix
  2. Using docker run in a step instead of services (conditionally based on matrix.driver)

This is a minor optimization and fine to defer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 78 - 106, The workflow currently
starts both services "postgres" and "neo4j" for every matrix entry which wastes
resources; change the workflow so only the needed DB runs per matrix.driver by
either (A) splitting into separate jobs (e.g., a job for postgres tests and
another for neo4j tests) and assigning the appropriate service in each job, or
(B) remove the services section and conditionally start the required container
with docker run inside a step that checks matrix.driver (use matrix.driver to
decide whether to docker run postgres:16 or neo4j:4.4.42); update job
dependencies and health/wait logic accordingly so tests run only after the
selected DB is ready.

92-93: Named volumes may not work as expected in GitHub Actions services.

GitHub Actions service containers are ephemeral and volumes declared this way don't persist between workflow runs. The pg_data and neo4j_data volumes are unnecessary here since tests start with cleanDB anyway and don't rely on persistence.

Consider removing the volume declarations to avoid potential confusion or unexpected behavior.

♻️ Proposed simplification
         options: >-
           --health-cmd "pg_isready -U dawgs -d dawgs"
           --health-interval 10s
           --health-timeout 5s
           --health-retries 5
-        volumes:
-          - pg_data:/var/lib/postgresql/data
       neo4j:
         image: neo4j:4.4.42
         env:
           NEO4J_AUTH: neo4j/weneedbetterpasswords
         ports:
           - 7687:7687
         options: >-
           --health-cmd "curl -f http://localhost:7474"
           --health-interval 10s
           --health-timeout 5s
           --health-retries 5
-        volumes:
-          - neo4j_data:/data

Also applies to: 105-106

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 92 - 93, The workflow defines
named service volumes (volumes: - pg_data:/var/lib/postgresql/data and
neo4j_data) which are unnecessary for ephemeral GitHub Actions service
containers and may mislead; remove the volume declarations referencing pg_data
and neo4j_data from the service definitions in the workflow file so the
Postgres/Neo4j services run without persisted volumes (tests rely on cleanDB),
leaving any other service config intact.

24-24: Remove temporary branch trigger before merging.

The PQE-401 branch was added to test this workflow. Remember to remove it before merging to main, or the workflow will continue triggering on that branch pattern after the PR is merged.

♻️ Proposed removal
   pull_request:
     branches:
       - "main"
       - "stage/**"
-      - "PQE-401"
     types:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml at line 24, The workflow is still configured
to trigger on a temporary branch pattern "PQE-401"; remove that branch entry
from the triggers in the workflow file so the workflow no longer listens for
that temporary branch after merge. Locate the literal "PQE-401" in the
workflow’s trigger list (the value currently in the array under on:
push/branches or similar) and delete that string entry, leaving only the
intended branch patterns (e.g., main) in the branches array.
docker-compose.yml (1)

1-25: Consider adding health checks for local development consistency.

The GitHub Actions workflow defines health checks for both services, but this local docker-compose.yml does not. Adding health checks here would ensure consistent behavior between local and CI environments, especially when scripts wait for services to be ready.

♻️ Proposed health checks for local parity
 services:
   postgres:
     image: postgres:16
     environment:
       POSTGRES_USER: dawg
       POSTGRES_PASSWORD: weneedbetterpasswords
       POSTGRES_DB: dawg
     ports:
       - "65432:5432"
     volumes:
       - pg_data:/var/lib/postgresql/data
+    healthcheck:
+      test: ["CMD", "pg_isready", "-U", "dawg", "-d", "dawg"]
+      interval: 10s
+      timeout: 5s
+      retries: 5

   neo4j:
     image: neo4j:4.4.42
     environment:
       NEO4J_AUTH: neo4j/weneedbetterpasswords
     ports:
       - "7687:7687"
       - "7474:7474"
     volumes:
       - neo4j_data:/data
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:7474"]
+      interval: 10s
+      timeout: 5s
+      retries: 5
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 1 - 25, Add Docker healthchecks to the
postgres and neo4j services so local compose matches CI: for the postgres
service (where POSTGRES_USER/POSTGRES_PASSWORD/POSTGRES_DB are set) add a
healthcheck that runs pg_isready against port 5432 with a sensible
interval/timeout/retries; for the neo4j service (where NEO4J_AUTH is set) add a
healthcheck that probes the HTTP port 7474 (or Bolt 7687) using a simple curl or
bolt/cypher check with basic auth, and set interval/timeout/retries so scripts
that wait for healthy status behave the same locally as in GitHub Actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ops/ops_test.go`:
- Around line 140-157: The WriteTransaction that calls DeleteRelationships on
testSuite.db ignores its returned error; add a check after the call to
testSuite.db.WriteTransaction (the block invoking DeleteRelationships) and fail
the test if err != nil (use require.NoError or require.Nil with t) so deletion
failures don't go unnoticed; keep the subsequent ReadTransaction and assertions
unchanged but only run them when the write error is nil.
- Around line 63-77: The Neo4j branch opens the database with dawgs.Open
(neo4j.DriverName) but never calls db.AssertSchema like the Postgres branch
does; after successfully creating db in the case "neo4j","bolt" block (after the
dawgs.Open and require.NoError(t, err)), add a call to
db.AssertSchema(context.Background(), testSchema) and handle the error similarly
to the Postgres branch (require.NoError) so the test schema is ensured for
Neo4j; reference symbols: case "neo4j","bolt", u.Scheme, connStr, dawgs.Open,
neo4j.DriverName, db, db.AssertSchema, testSchema.

---

Nitpick comments:
In @.github/workflows/go-test.yml:
- Around line 78-106: The workflow currently starts both services "postgres" and
"neo4j" for every matrix entry which wastes resources; change the workflow so
only the needed DB runs per matrix.driver by either (A) splitting into separate
jobs (e.g., a job for postgres tests and another for neo4j tests) and assigning
the appropriate service in each job, or (B) remove the services section and
conditionally start the required container with docker run inside a step that
checks matrix.driver (use matrix.driver to decide whether to docker run
postgres:16 or neo4j:4.4.42); update job dependencies and health/wait logic
accordingly so tests run only after the selected DB is ready.
- Around line 92-93: The workflow defines named service volumes (volumes: -
pg_data:/var/lib/postgresql/data and neo4j_data) which are unnecessary for
ephemeral GitHub Actions service containers and may mislead; remove the volume
declarations referencing pg_data and neo4j_data from the service definitions in
the workflow file so the Postgres/Neo4j services run without persisted volumes
(tests rely on cleanDB), leaving any other service config intact.
- Line 24: The workflow is still configured to trigger on a temporary branch
pattern "PQE-401"; remove that branch entry from the triggers in the workflow
file so the workflow no longer listens for that temporary branch after merge.
Locate the literal "PQE-401" in the workflow’s trigger list (the value currently
in the array under on: push/branches or similar) and delete that string entry,
leaving only the intended branch patterns (e.g., main) in the branches array.

In `@docker-compose.yml`:
- Around line 1-25: Add Docker healthchecks to the postgres and neo4j services
so local compose matches CI: for the postgres service (where
POSTGRES_USER/POSTGRES_PASSWORD/POSTGRES_DB are set) add a healthcheck that runs
pg_isready against port 5432 with a sensible interval/timeout/retries; for the
neo4j service (where NEO4J_AUTH is set) add a healthcheck that probes the HTTP
port 7474 (or Bolt 7687) using a simple curl or bolt/cypher check with basic
auth, and set interval/timeout/retries so scripts that wait for healthy status
behave the same locally as in GitHub Actions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3140a6c9-ee5c-49d7-9a38-712503c2653d

📥 Commits

Reviewing files that changed from the base of the PR and between b3c5ab8 and f9cc9c4.

📒 Files selected for processing (3)
  • .github/workflows/go-test.yml
  • docker-compose.yml
  • ops/ops_test.go

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
.github/workflows/go-test.yml (2)

74-77: Consider using GitHub secrets for credentials.

The connection strings contain hardcoded passwords. While these are ephemeral CI containers, using GitHub secrets is a cleaner approach and avoids static analysis warnings (Checkov CKV_SECRET_4).

Additionally, the environment variable Driver_CONNECTION_STRING (line 116) uses mixed case. Convention is typically SCREAMING_SNAKE_CASE for environment variables (e.g., DRIVER_CONNECTION_STRING).

Example using secrets
# In repository secrets, define:
# - TEST_PG_PASSWORD
# - TEST_NEO4J_PASSWORD

matrix:
  include:
    - driver: pg
      connection: postgres://dawgs:${{ secrets.TEST_PG_PASSWORD }}@localhost:5432/dawgs?sslmode=disable
    - driver: neo4j
      connection: bolt://neo4j:${{ secrets.TEST_NEO4J_PASSWORD }}@localhost:7687
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 74 - 77, Replace the hardcoded
passwords in the matrix "connection" values for the job entries that set
driver/connection (the pg and neo4j rows) to use GitHub Actions secrets (e.g.,
secrets.TEST_PG_PASSWORD and secrets.TEST_NEO4J_PASSWORD) instead of plain-text
passwords, and rename the environment variable key currently referenced as
Driver_CONNECTION_STRING to use SCREAMING_SNAKE_CASE (DRIVER_CONNECTION_STRING)
throughout the workflow to follow env var conventions and avoid casing
mismatches.

78-102: Both service containers start for every matrix entry.

With the current setup, both Postgres and Neo4j containers are started for every matrix entry, even though only one is used per run. This doubles the resource consumption.

Consider either:

  1. Splitting into two separate jobs with explicit service definitions
  2. Using conditional service definitions (though GitHub Actions has limited support for this)
Option 1: Separate jobs
  integration-test-pg:
    name: Run integration tests (pg)
    runs-on: ubuntu-latest
    services:
      postgres:
        image: postgres:16
        # ... postgres config only
    steps:
      # ...

  integration-test-neo4j:
    name: Run integration tests (neo4j)
    runs-on: ubuntu-latest
    services:
      neo4j:
        image: neo4j:4.4.42
        # ... neo4j config only
    steps:
      # ...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 78 - 102, The workflow currently
starts both service containers (services: postgres and neo4j) for every matrix
entry, doubling resource use; split this into two jobs each defining only the
required service (e.g., create jobs like integration-test-pg that defines only
the postgres service and integration-test-neo4j that defines only the neo4j
service) or gate service startup with job-level conditionals tied to the matrix
value (use job "if" checks such as if: matrix.database == 'postgres' to only
declare the postgres service, and similarly for neo4j); update any step
references to the service host/port accordingly so tests run against the correct
service.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/go-test.yml:
- Line 24: Remove the temporary branch trigger string "PQE-401" from the
workflow's branches trigger (the branches array under on.push/on.pull_request)
so the feature-branch-specific trigger is not included in the final workflow;
update the branches array to only include the intended long-term branch names
(e.g., main) and ensure no other temporary branch names remain.
- Around line 98-102: The health check uses curl which is not present in the
neo4j:4.4.42 image; update the Docker Compose/GitHub Actions service options
(the options key / --health-cmd) to use a tool present in the image—either
replace the curl command with a wget-based check (e.g., use wget -q --spider
http://localhost:7474/ || exit 1) or use cypher-shell to run a simple query
(e.g., cypher-shell -a bolt://localhost:7687 -u neo4j -p "$NEO4J_PASSWORD"
'RETURN 1' >/dev/null 2>&1 || exit 1) and keep the same --health-interval,
--health-timeout and --health-retries settings.

---

Nitpick comments:
In @.github/workflows/go-test.yml:
- Around line 74-77: Replace the hardcoded passwords in the matrix "connection"
values for the job entries that set driver/connection (the pg and neo4j rows) to
use GitHub Actions secrets (e.g., secrets.TEST_PG_PASSWORD and
secrets.TEST_NEO4J_PASSWORD) instead of plain-text passwords, and rename the
environment variable key currently referenced as Driver_CONNECTION_STRING to use
SCREAMING_SNAKE_CASE (DRIVER_CONNECTION_STRING) throughout the workflow to
follow env var conventions and avoid casing mismatches.
- Around line 78-102: The workflow currently starts both service containers
(services: postgres and neo4j) for every matrix entry, doubling resource use;
split this into two jobs each defining only the required service (e.g., create
jobs like integration-test-pg that defines only the postgres service and
integration-test-neo4j that defines only the neo4j service) or gate service
startup with job-level conditionals tied to the matrix value (use job "if"
checks such as if: matrix.database == 'postgres' to only declare the postgres
service, and similarly for neo4j); update any step references to the service
host/port accordingly so tests run against the correct service.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0da95224-bcda-4dd1-b430-d7a3cd55b6f0

📥 Commits

Reviewing files that changed from the base of the PR and between f9cc9c4 and 69977e2.

📒 Files selected for processing (1)
  • .github/workflows/go-test.yml

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
.github/workflows/go-test.yml (1)

24-24: ⚠️ Potential issue | 🟡 Minor

Remove PR-specific branch trigger before merge.

Line 24 still includes PQE-401, which keeps this workflow tied to a feature branch after merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml at line 24, Remove the PR-specific branch
trigger string "PQE-401" from the workflow's branch trigger list so the GitHub
Actions workflow is not tied to that feature branch after merge; locate the
branch array where "PQE-401" is present (the branch trigger entries) and delete
that entry (or replace it with the intended persistent branch names like main)
to ensure the workflow triggers only on the correct branches.
🧹 Nitpick comments (1)
.github/workflows/go-test.yml (1)

75-77: Avoid embedding credentials directly in workflow YAML.

Even for test containers, hardcoded credentials here trigger secret-scanning noise and can be copied into less safe contexts. Prefer job-level env vars (or repo variables) and reference them in both service env and matrix connection strings.

Also applies to: 82-85, 95-96

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 75 - 77, Replace hardcoded DB
credentials in the GitHub Actions workflow by moving secrets into job-level
environment variables and referencing those vars in service env and matrix
connection strings; e.g., define env entries like POSTGRES_USER,
POSTGRES_PASSWORD, NEO4J_USER, NEO4J_PASSWORD at the job level and then update
the connection values (the postgres "connection: postgres://..." and neo4j
"connection: bolt://...") and any other matrix entries to use the env variables
(e.g.,
postgres://$POSTGRES_USER:$POSTGRES_PASSWORD@localhost:5432/dawgs?sslmode=disable)
or use ${{ secrets.X }} if you prefer repo secrets, and do the same for the
other occurrences mentioned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/go-test.yml:
- Around line 78-103: The workflow currently starts both services (services:
postgres and neo4j) for every matrix entry, causing matrix legs to interfere;
fix by splitting the work into separate jobs (e.g., test-postgres and
test-neo4j) or by making separate matrix jobs per backend so each job's services
include only the needed service. Update the job that currently defines both
postgres and neo4j (the go-test job / strategy.matrix) to instead create two
jobs—one with only the postgres service and one with only the neo4j service—and
reuse the same steps/steps template so each matrix leg runs isolated against its
intended backend (reference symbols: services, postgres, neo4j, strategy.matrix,
go-test job name). Ensure tests/steps still read the matrix variables (driver)
or adjust matrix entries per job accordingly.
- Around line 76-77: The Neo4j test matrix is using the wrong URL scheme
("bolt://...") which fails the scheme validation in drivers/neo4j/neo4j.go that
expects "neo4j"; update the connection URL in the matrix to use the "neo4j://"
scheme (e.g., change bolt://neo4j:password@localhost:7687 to
neo4j://neo4j:password@localhost:7687) so the scheme check in the code passes
and the Neo4j test leg can initialize correctly.

---

Duplicate comments:
In @.github/workflows/go-test.yml:
- Line 24: Remove the PR-specific branch trigger string "PQE-401" from the
workflow's branch trigger list so the GitHub Actions workflow is not tied to
that feature branch after merge; locate the branch array where "PQE-401" is
present (the branch trigger entries) and delete that entry (or replace it with
the intended persistent branch names like main) to ensure the workflow triggers
only on the correct branches.

---

Nitpick comments:
In @.github/workflows/go-test.yml:
- Around line 75-77: Replace hardcoded DB credentials in the GitHub Actions
workflow by moving secrets into job-level environment variables and referencing
those vars in service env and matrix connection strings; e.g., define env
entries like POSTGRES_USER, POSTGRES_PASSWORD, NEO4J_USER, NEO4J_PASSWORD at the
job level and then update the connection values (the postgres "connection:
postgres://..." and neo4j "connection: bolt://...") and any other matrix entries
to use the env variables (e.g.,
postgres://$POSTGRES_USER:$POSTGRES_PASSWORD@localhost:5432/dawgs?sslmode=disable)
or use ${{ secrets.X }} if you prefer repo secrets, and do the same for the
other occurrences mentioned.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 50e616ad-2b15-411d-8bb8-956607f5ac6c

📥 Commits

Reviewing files that changed from the base of the PR and between 69977e2 and c8c8db6.

📒 Files selected for processing (1)
  • .github/workflows/go-test.yml

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/go-test.yml (1)

77-102: ⚠️ Potential issue | 🟠 Major

Matrix legs are still coupled by always starting both databases.

At Line [77], both postgres and neo4j services are started for every matrix run. This means the pg leg can fail on Neo4j startup/health (and vice versa), which hurts reliability and adds avoidable runtime.

Suggested direction (split into per-backend jobs)
- integration-test:
-   strategy:
-     matrix:
-       include:
-         - driver: pg
-           connection: postgres://...
-         - driver: neo4j
-           connection: bolt://...
-   services:
-     postgres: ...
-     neo4j: ...
+ integration-test-pg:
+   services:
+     postgres: ...
+   env:
+     Driver_CONNECTION_STRING: postgres://...
+   steps:
+     - run: go test -tags integration -v ./ops/...
+
+ integration-test-neo4j:
+   services:
+     neo4j: ...
+   env:
+     Driver_CONNECTION_STRING: bolt://...
+   steps:
+     - run: go test -tags integration -v ./ops/...
In GitHub Actions, when a job uses a matrix and defines multiple job-level services, are all services started for every matrix entry, or can services be skipped per entry without splitting jobs?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-test.yml around lines 77 - 102, The workflow currently
starts both services ("postgres" and "neo4j") for every matrix entry, coupling
matrix legs and letting one DB's startup/health affect the other; split the runs
so each matrix/job only starts the service it needs by removing the global
services block that lists both and instead create separate jobs (or matrix
include entries) per backend (e.g., a job named test-postgres that defines
services: postgres and a job named test-neo4j that defines services: neo4j) and
drive them with strategy.matrix (or separate job entries) so "postgres" and
"neo4j" are started only for their respective jobs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ops/ops_test.go`:
- Around line 65-67: The scheme rewrite for bolt is currently case-sensitive
(u.Scheme == "bolt") so mixed-case inputs like "BOLT://..." won't be rewritten;
update the check in ops_test.go to compare case-insensitively (e.g., use
strings.EqualFold(u.Scheme, "bolt") or strings.ToLower(u.Scheme) == "bolt")
before setting u.Scheme = "neo4j" and recomputing connStr; also add the strings
import if not present.

---

Duplicate comments:
In @.github/workflows/go-test.yml:
- Around line 77-102: The workflow currently starts both services ("postgres"
and "neo4j") for every matrix entry, coupling matrix legs and letting one DB's
startup/health affect the other; split the runs so each matrix/job only starts
the service it needs by removing the global services block that lists both and
instead create separate jobs (or matrix include entries) per backend (e.g., a
job named test-postgres that defines services: postgres and a job named
test-neo4j that defines services: neo4j) and drive them with strategy.matrix (or
separate job entries) so "postgres" and "neo4j" are started only for their
respective jobs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: abee189d-336b-4dd1-996a-a28f4741228f

📥 Commits

Reviewing files that changed from the base of the PR and between c8c8db6 and 1f11e56.

📒 Files selected for processing (2)
  • .github/workflows/go-test.yml
  • ops/ops_test.go

@ykaiboussiSO ykaiboussiSO merged commit 2380370 into main Apr 1, 2026
4 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