(chore): Change to use a shared connection string for simplicity testing PQE-411

[kpom-specter]

Description

Resolves: PQE-411

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

Release Notes

• Refactor
  • Updated test configuration to use environment variables instead of CLI flags for database connection settings.
  • Driver type is now automatically detected from the CONNECTION_STRING environment variable, eliminating the need for manual flag specification during test execution.

[coderabbitai]

Walkthrough

Removes CLI flags for driver selection and replaces them with environment variable-based configuration. A new driverFromConnStr function parses the CONNECTION_STRING environment variable as a URL to automatically detect the database driver at runtime, eliminating manual flag specification during test setup.

Changes

Cohort / File(s)

Summary

Test Infrastructure Configuration integration/cypher_test.go, integration/harness.go

Removed driverFlag and connStrFlag exported variables. Added driverFromConnStr(connStr string) helper that parses connection URLs and maps schemes (postgresql → pg.DriverName, neo4j → neo4j.DriverName) to driver names. Updated SetupDB to detect driver from CONNECTION_STRING environment variable instead of CLI flags, failing immediately if detection errors or variable is unset. PostgreSQL pool creation now conditional on detected driver match instead of flag value.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Away with flags, we hop with glee!
Environment whispers our destiny,
Connection strings dance, schemes align—
PostgreSQL or Neo4j divine!
Our harness springs forth, light and free! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

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.
Description check	⚠️ Warning	The PR description contains only the template structure with no substantive content filled in. All checkboxes remain unchecked, the description section is empty, and testing/checklist items are not addressed.	Complete the description section explaining the motivation for switching to environment-based connection strings. Select appropriate change type(s), check relevant testing boxes, and address checklist items. Indicate driver impact if applicable.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: switching from CLI flags to environment-based connection string detection for database driver selection.

_{✏️ 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 kpom/connection_strings_over_flags

---

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

[coderabbitai]

🧹 Nitpick comments (1)

integration/cypher_test.go (1)

95-98: Consider removing duplicate driver derivation from the test.

SetupDB already resolves/validates the driver path; re-reading CONNECTION_STRING here duplicates setup logic. Returning driver from SetupDB would keep this in one place.

♻️ Possible refactor direction

- db, ctx := SetupDB(t, datasetNames...)
- driver, err := driverFromConnStr(os.Getenv("CONNECTION_STRING"))
- if err != nil {
-     t.Fatalf("Failed to detect driver: %v", err)
- }
+ db, ctx, driver := SetupDB(t, datasetNames...)

(Then adjust SetupDB signature in integration/harness.go to return driver.)

🤖 Prompt for AI Agents

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

In `@integration/cypher_test.go` around lines 95 - 98, The test duplicates driver
detection by calling driverFromConnStr(os.Getenv("CONNECTION_STRING")) even
though SetupDB already resolves the driver; change SetupDB (in
integration/harness.go) to return the detected driver (e.g., add a return value
like driver string) and update its signature, then remove the duplicate
driverFromConnStr call from integration/cypher_test.go and use the driver
returned from SetupDB; also update any other callers of SetupDB to accept and
propagate the new return value.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@integration/cypher_test.go`:
- Around line 95-98: The test duplicates driver detection by calling
driverFromConnStr(os.Getenv("CONNECTION_STRING")) even though SetupDB already
resolves the driver; change SetupDB (in integration/harness.go) to return the
detected driver (e.g., add a return value like driver string) and update its
signature, then remove the duplicate driverFromConnStr call from
integration/cypher_test.go and use the driver returned from SetupDB; also update
any other callers of SetupDB to accept and propagate the new return value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ea6f0c0f-55c3-4f97-8401-6f698d6c4fbe

📥 Commits

Reviewing files that changed from the base of the PR and between 817cd6b and 6e52a4f.

📒 Files selected for processing (2)

• integration/cypher_test.go
• integration/harness.go

[ykaiboussiSO]

LGTM