test: Phase 3 — boost main package coverage to 82.8%

[AWaterColorPen]

Summary

Phase 3 first step: push the main package test coverage from 79.1% to 82.8% (goal: 80%+).

What changed

Added coverage_boost_test.go with targeted tests for previously uncovered functions:

Function	File
Clients.SetLogger	client.go
Clients.BuildSQL	client.go
Manager.SetLogger	manager.go
Manager.BuildSQL	manager.go
NewTranslator (direct-SQL path)	dictionary_translator.go
FileAdapter.GetMetricsBySource	dictionary_adapter.go
FileAdapter.GetDimensionsBySource	dictionary_adapter.go
DBOption.NewDB unsupported-type error path	database.go

Coverage before / after

before: coverage: 79.1% of statements
after:  coverage: 82.8% of statements

Notes

• No production code was modified; this is test-only.
• All existing tests continue to pass.
• Part of the Phase 3 modernisation plan (dependency upgrades + 80%+ coverage).

[AWaterColorPen]

**Misleading setup: MockLoad(m) here loads data into m's internal DB, but clients was created from a separate newClients(t.TempDir()) with a different SQLite file. The loaded data is never visible to clients.

Since BuildSQL only generates a SQL string and never executes, the test still passes — but the MockLoad(m) call is dead weight that implies a dependency that does not exist. Consider either:

• Removing MockLoad(m) (and the manager entirely), or
• Simplifying to sql, err := m.BuildSQL(query) directly (which also tests Manager.BuildSQL — though that is covered by TestManager_BuildSQL below).

[AWaterColorPen]

Variable naming + weak assertion. The variable sql is a types.Clause, not a SQL string — calling it sql is confusing. Suggest renaming to clause.

More importantly, assert.NotNil(t, clause) does not verify that the direct-SQL path actually preserved query.Sql. Since the whole point of this test is to cover the query.Sql != "" branch, consider also asserting that the clause carries the original SQL:

clause, err := dict.Translate(query)
assert.NoError(t, err)
assert.NotNil(t, clause)
// verify the direct-SQL path preserved the raw SQL
if sc, ok := clause.(*types.SqlClause); ok {
    assert.Equal(t, "SELECT 1", sc.Sql)
} else {
    t.Errorf("expected *types.SqlClause, got %T", clause)
}

This makes the test actually exercise what it claims to test.

[AWaterColorPen]

Good coverage boost — all seven target code paths are now reached, and the helper functions (newDictionary, newFileAdapter) are clean and well-scoped.

Two things to fix before merging:

1. TestClients_BuildSQL — MockLoad(m) is dead code here (loads data into manager's DB, not the separate clients instance). Remove it, or simplify by calling m.BuildSQL(query) directly.

2. TestNewTranslator_DirectSQL — the result variable is named sql but holds a types.Clause; and the assertion (NotNil) does not verify the direct-SQL path actually preserved the raw SQL string. Please rename and add a type assertion to check clause.(*types.SqlClause).Sql == "SELECT 1".

Everything else looks correct: TestDBOption_NewDB_UnsupportedType correctly targets the default branch in getDialect, the SetLogger smoke tests are sufficient given the trivial implementations, and TestFileAdapter_GetMetricsBySource/GetDimensionsBySource appropriately test both the happy path and the empty-result path.

[AWaterColorPen]

Both review items addressed in commit 56d672f:

1. TestClients_BuildSQL — removed the dead MockLoad(m) call; added a comment explaining that BuildSQL uses DryRun mode and does not need data loading.
2. TestNewTranslator_DirectSQL — renamed sql to clause, added a type assertion clause.(*types.SqlClause), and an explicit equality check assert.Equal(t, "SELECT 1", sqlClause.Sql).

All tests pass at 82.8% coverage.