test: connections — dbt-profiles, Docker, MongoDB auth

[anandgupta42]

What does this PR do?

Adds 12 new tests to the connection subsystem covering previously untested code paths identified during proactive test discovery. All tests are pure/deterministic with no external dependencies.

1. parseDbtProfiles — src/altimate/native/connections/dbt-profiles.ts (5 new tests)

The dbt profiles parser had basic coverage (env_var resolution, single profile parsing) but missed several real-world edge cases. New coverage includes:

• env_var() with default values: Users commonly write {{ env_var('SECRET', 'dev_fallback') }} in profiles.yml. The regex captures the optional second group, but this was untested — a refactor could silently break default resolution, causing empty strings instead of fallback values.
• Spark adapter → Databricks mapping: The ADAPTER_TYPE_MAP maps spark to databricks. Users with Spark-flavored dbt projects would get "unsupported type" errors if this alias were removed.
• Trino adapter → Postgres mapping: Similarly, trino maps to postgres (wire-compatible). Untested non-trivial alias.
• config key skipping: Real profiles.yml files often have a top-level config: key for global dbt settings. The parser explicitly skips it (profileName === "config"), but this guard had no test — removing it would create phantom warehouse entries.
• Multiple profiles × multiple outputs: All existing tests used single-profile YAMLs. This test exercises the nested iteration (2 profiles × 2 outputs = 4 connections) to catch off-by-one or early-break bugs.

2. dbtConnectionsToConfigs — src/altimate/native/connections/dbt-profiles.ts (2 new tests)

This function converts the parsed connection array into a keyed Record used by the auto-discovery pipeline. Zero tests existed. Coverage includes basic conversion and empty-input handling.

3. containerToConfig — src/altimate/native/connections/docker-discovery.ts (2 new tests)

This exported function converts Docker-discovered containers into ConnectionConfig objects. Previously only discoverContainers (which returns [] without dockerode) was tested. New coverage includes:

• Full config construction with all fields populated
• Verification that optional fields (user, password, database) are omitted from the object (not set to undefined) when the container doesn't provide them — the distinction matters for driver serialization

4. detectAuthMethod MongoDB branch — src/altimate/native/connections/registry.ts (2 new tests)

MongoDB support was added in #482 but the detectAuthMethod telemetry helper had no MongoDB-specific tests. The MongoDB branch at line 229 is only reachable when password is falsy (the generic password check at line 227 fires first). New tests cover:

• { type: "mongodb" } → "connection_string"
• { type: "mongo" } → "connection_string" (alias)

Critic review note: A test for detectAuthMethod({ type: "mongodb", password: "secret" }) was proposed but rejected by the critic agent — the MongoDB-specific branch is unreachable in that case (the generic password check returns first). Only the actually-reachable branch is tested.

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage from automated test discovery

How did you verify your code works?

bun test test/altimate/connections.test.ts   # 50 pass (38 existing + 12 new)

Marker check passes (bun run script/upstream/analyze.ts --markers --base main --strict).

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Summary by CodeRabbit

• Tests
  • Expanded test coverage for DBT profile parsing, including environment variable resolution, adapter mapping, and multi-profile handling
  • Added tests for Docker/container connection configuration conversion with proper field handling
  • Added tests for MongoDB authentication method detection

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Expands test coverage for DBT profile parsing edge cases, Docker container connection conversion via containerToConfig, connection array-to-object transformation via dbtConnectionsToConfigs, and registry functions including detectAuthMethod for MongoDB authentication detection.

Changes

Cohort / File(s)	Summary
Test Coverage Expansion packages/opencode/test/altimate/connections.test.ts	Added comprehensive test blocks for DBT profile parsing (env_var resolution, adapter mapping, multiple profiles), dbtConnectionsToConfigs array-to-object conversion, containerToConfig Docker connection handling with optional user/password/database fields, and MongoDB auth method detection via detectAuthMethod.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: eliminate 29 pre-existing test failures in CI #411: Modifies the same test file (packages/opencode/test/altimate/connections.test.ts), adding concurrent test coverage for DuckDB readiness and connector-API validation alongside other connection-related tests.

Suggested labels

contributor

Poem

🐰 Hoppy tests hop through the code,
Parsing profiles down the road,
Containers and configs aligned,
Edge cases caught—none left behind!
Coverage grows with each new bound, ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding tests for connections subsystem covering dbt-profiles, Docker, and MongoDB auth.
Description check	✅ Passed	The PR description provides comprehensive detail on what was added, why each test matters, test coverage breakdown, and verification steps. All template sections (Summary, Test Plan, Checklist) are present and complete.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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 test/hourly-20260327-1410

---

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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/opencode/test/altimate/connections.test.ts (1)

272-303: Test logic is correct; consider using tmpdir fixture for consistency with guidelines.

The env_var default resolution test correctly validates the fallback behavior. However, the coding guidelines recommend using tmpdir from fixture/fixture.ts with await using syntax for automatic cleanup.

While the current pattern matches existing tests in this file, consider refactoring to align with the guidelines:

♻️ Optional refactor using tmpdir fixture

test("env_var with default value resolves to default when env var is missing", async () => {
  await using tmp = await tmpdir()
  delete process.env.DBT_TEST_NONEXISTENT_VAR

  const profilesPath = path.join(tmp.path, "profiles.yml")
  fs.writeFileSync(profilesPath, `
myproject:
  outputs:
    dev:
      type: postgres
      host: localhost
      password: "{{ env_var('DBT_TEST_NONEXISTENT_VAR', 'fallback_pw') }}"
      dbname: testdb
`)

  const connections = await parseDbtProfiles(profilesPath)
  expect(connections).toHaveLength(1)
  expect(connections[0].config.password).toBe("fallback_pw")
})

As per coding guidelines: "Use the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup" and "Always use await using syntax with tmpdir() for automatic cleanup".

🤖 Prompt for AI Agents

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

In `@packages/opencode/test/altimate/connections.test.ts` around lines 272 - 303,
The test "env_var with default value resolves to default when env var is
missing" correctly asserts fallback behavior but should use the tmpdir fixture
for automatic cleanup; replace the manual fs.mkdtempSync/fs.rmSync pattern with
the tmpdir fixture from fixture/fixture.ts using "await using tmp = await
tmpdir()", build profilesPath from tmp.path, write the file (keep
parseDbtProfiles and process.env deletion unchanged), and remove the try/finally
cleanup block so the fixture handles teardown.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@packages/opencode/test/altimate/connections.test.ts`:
- Line 16: The import list in the test currently includes
categorizeConnectionError which is unused; either remove
categorizeConnectionError from the import statement (leaving detectAuthMethod)
or add tests that exercise the categorizeConnectionError function (call
categorizeConnectionError with representative error inputs and assert expected
categories), ensuring the test actually references the function so the import is
used.

---

Nitpick comments:
In `@packages/opencode/test/altimate/connections.test.ts`:
- Around line 272-303: The test "env_var with default value resolves to default
when env var is missing" correctly asserts fallback behavior but should use the
tmpdir fixture for automatic cleanup; replace the manual
fs.mkdtempSync/fs.rmSync pattern with the tmpdir fixture from fixture/fixture.ts
using "await using tmp = await tmpdir()", build profilesPath from tmp.path,
write the file (keep parseDbtProfiles and process.env deletion unchanged), and
remove the try/finally cleanup block so the fixture handles teardown.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: eb84ec21-cd2a-433c-834e-3aa2a739b88e

📥 Commits

Reviewing files that changed from the base of the PR and between abcaa1d and a4cc3e6.

📒 Files selected for processing (1)

• packages/opencode/test/altimate/connections.test.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unused import: categorizeConnectionError

This function is imported but never used in any test. Either add tests for it or remove the import.

-import { detectAuthMethod, categorizeConnectionError } from "../../src/altimate/native/connections/registry"
+import { detectAuthMethod } from "../../src/altimate/native/connections/registry"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	import { detectAuthMethod, categorizeConnectionError } from "../../src/altimate/native/connections/registry"
	import { detectAuthMethod } from "../../src/altimate/native/connections/registry"

🤖 Prompt for AI Agents

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

In `@packages/opencode/test/altimate/connections.test.ts` at line 16, The import
list in the test currently includes categorizeConnectionError which is unused;
either remove categorizeConnectionError from the import statement (leaving
detectAuthMethod) or add tests that exercise the categorizeConnectionError
function (call categorizeConnectionError with representative error inputs and
assert expected categories), ensuring the test actually references the function
so the import is used.

[anandgupta42]

Consolidated into #514. This PR's test coverage was either already on main or has been included in the consolidated PR.