Skip to content

Commit fabb764

Browse files
committed
fix(doctor): serialize osEnviron hook for race-safe parallel tests
1 parent d6ac485 commit fabb764

4 files changed

Lines changed: 33 additions & 13 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ Percentages are **statement coverage** from a merged profile (`go test -coverpro
797797
| `config` | 73.2% | 339 LOC | High |
798798
| `plugin/host` | 62.5% | 623 LOC | Medium |
799799
| `client/config` | 58.0% | 1993 LOC | Medium |
800-
| `internal/doctor` | 52.1% | 802 LOC | Medium |
800+
| `internal/doctor` | 52.5% | 809 LOC | Medium |
801801
| `plugin/store` | 47.0% | 552 LOC | Medium |
802802
| `cmd/license` | 42.2% | 160 LOC | Medium |
803803
| `server` | 36.3% | 7215 LOC | Low |
@@ -806,7 +806,7 @@ Percentages are **statement coverage** from a merged profile (`go test -coverpro
806806
| `client` | 23.1% | 5555 LOC | Low |
807807
| `cmd/server` | 13.7% | 484 LOC | Low |
808808

809-
**Overall: 37.8%** (main module packages only). See [TESTING.md](TESTING.md) for detailed information.
809+
**Overall: 37.9%** (main module packages only). See [TESTING.md](TESTING.md) for detailed information.
810810

811811
## Contributing
812812

TESTING.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ The Marchat test suite provides foundational coverage of the application's core
1212
- **Database Tests**: Testing database operations and schema management
1313
- **Server Tests**: Testing WebSocket handling, message routing, and user management
1414

15-
**Note**: When **`main`** has moved past the latest Git tag, a narrative summary may appear in **README.md****Latest Updates**. This is a foundational test suite with good coverage for smaller utility packages and significantly improved coverage for client and server components. **Overall statement coverage is 37.8%** across all packages in the main module, computed from the merged profile at the repo root (for example the `coverage` file or another path passed to `go test -coverprofile=... ./...`). Regenerate summaries with `go tool cover -func=<same-path>`. On **Windows PowerShell**, prefer a profile filename **without** a `.out` suffix (e.g. `mergedcoverage` or `coverage`) so the argument is not misparsed.
15+
**Note**: When **`main`** has moved past the latest Git tag, a narrative summary may appear in **README.md****Latest Updates**. This is a foundational test suite with good coverage for smaller utility packages and significantly improved coverage for client and server components. **Overall statement coverage is 37.9%** across all packages in the main module, computed from the merged profile at the repo root (for example the `coverage` file or another path passed to `go test -coverprofile=... ./...`). Regenerate summaries with `go tool cover -func=<same-path>`. On **Windows PowerShell**, prefer a profile filename **without** a `.out` suffix (e.g. `mergedcoverage` or `coverage`) so the argument is not misparsed.
1616

1717
**Database backends:** Automated tests open **SQLite** (usually in-memory or a temp file). PostgreSQL and MySQL/MariaDB are supported at runtime via `MARCHAT_DB_PATH`. **GitHub Actions** runs an extra **`database-smoke`** job (see `.github/workflows/go.yml`) with Postgres 16 and MySQL 8 service containers: it sets `MARCHAT_CI_POSTGRES_URL` and `MARCHAT_CI_MYSQL_URL` and runs `TestPostgresInitDBAndSchemaSmoke` / `TestMySQLInitDBAndSchemaSmoke` in `server/db_ci_smoke_test.go` (`InitDB` + `CreateSchema` + table checks). Locally, those tests **skip** unless you export the same variables (for MySQL, use a `mysql:` or `mysql://` prefix on the DSN so it is not parsed as a SQLite path). Schema creation is dialect-aware (including MySQL/MariaDB rules for indexed text).
1818

@@ -209,7 +209,7 @@ go test -cover ./...
209209
| `config` | 73.2% | High | 339 | Small |
210210
| `plugin/host` | 62.5% | Medium | 623 | Medium |
211211
| `client/config` | 58.0% | Medium | 1993 | Medium |
212-
| `internal/doctor` | 52.1% | Medium | 802 | Medium |
212+
| `internal/doctor` | 52.5% | Medium | 809 | Medium |
213213
| `plugin/store` | 47.0% | Medium | 552 | Medium |
214214
| `cmd/license` | 42.2% | Medium | 160 | Small |
215215
| `server` | 36.3% | Low | 7215 | Large |
@@ -220,7 +220,7 @@ go test -cover ./...
220220

221221
¹Non-test `.go` files only, physical line count (`wc -l` style), **only `.go` files in that package directory** (not subpackages such as `client/config` inside `client/`). Regenerate with the Python snippet in **Test Metrics** or `find` + `wc`.
222222

223-
**Overall coverage: 37.8%** (all packages in the main module; merged profile `coverage` or another `-coverprofile` path)
223+
**Overall coverage: 37.9%** (all packages in the main module; merged profile `coverage` or another `-coverprofile` path)
224224

225225
### High Coverage (70%+)
226226
- **Shared Package**: Cryptographic operations, data types, message handling, version utilities (88.1%)
@@ -231,7 +231,7 @@ go test -cover ./...
231231
### Medium Coverage (40-70%)
232232
- **Plugin Host Package**: Load/start/stop lifecycle, JSON IPC with a minimal test plugin, `ExecuteCommand` (62.5%)
233233
- **Client Config Package**: Configuration management, path utilities, keystore migration, interactive UI (58.0%)
234-
- **Doctor Package**: Server/client diagnostics, env checks, update metadata, DB probes (52.1%)
234+
- **Doctor Package**: Server/client diagnostics, env checks, update metadata, DB probes (52.5%)
235235
- **Plugin Store**: Registry management, platform resolution, filtering, caching (47.0%)
236236
- **Command License**: CLI functions for license management (42.2%)
237237

@@ -249,6 +249,7 @@ Statement percentages below are from the merged profile (`go tool cover -func=co
249249
|------|----------|---------|-------------|
250250
| `shared/version.go` | 100.0% | shared | Version information functions |
251251
| `internal/doctor/db_checks.go` | 100.0% | internal/doctor | SQLite checks for `-doctor` |
252+
| `internal/doctor/env.go` | 97.4% | internal/doctor | Ordered `MARCHAT_*` env lines for doctor reports |
252253
| `client/file_picker.go` | 98.2% | client | File selection TUI component |
253254
| `server/health.go` | 89.3% | server | Health monitoring and status |
254255
| `plugin/license/validator.go` | 87.1% | plugin/license | License validation and verification |
@@ -425,8 +426,8 @@ When adding new functionality to Marchat:
425426
- **Top-level tests**: 348 `Test*` entrypoints from `go test -list . ./...` on the main module; the nested **`plugin/sdk`** module adds 10 more (`cd plugin/sdk && go test -list . ./...`).
426427
- **Test files**: 42 tracked `_test.go` files (`git ls-files '*_test.go'`), including `plugin/sdk/plugin_test.go` in the nested SDK module.
427428
- **Packages (`go list ./...`)**: 15 in the main module; `plugin/sdk` and `plugin/examples/echo` are nested modules with their own `go.mod` files (root `go test ./...` does not run their tests).
428-
- **Coverage by Package** (statement %, merged profile): 88.1% (`shared`), 87.1% (`plugin/license`), 80.3% (`client/crypto`), 73.2% (`config`), 62.5% (`plugin/host`), 58.0% (`client/config`), 52.1% (`internal/doctor`), 47.0% (`plugin/store`), 42.2% (`cmd/license`), 36.3% (`server`), 32.1% (`plugin/manager`), 24.1% (`client/exthook`), 23.1% (`client`), 13.7% (`cmd/server`)
429-
- **Overall Coverage**: **37.8%** across main-module packages (regenerate with `go test -coverprofile=mergedcoverage ./...` then `go tool cover -func=mergedcoverage`; on PowerShell avoid `-coverprofile=*.out`--see note above)
429+
- **Coverage by Package** (statement %, merged profile): 88.1% (`shared`), 87.1% (`plugin/license`), 80.3% (`client/crypto`), 73.2% (`config`), 62.5% (`plugin/host`), 58.0% (`client/config`), 52.5% (`internal/doctor`), 47.0% (`plugin/store`), 42.2% (`cmd/license`), 36.3% (`server`), 32.1% (`plugin/manager`), 24.1% (`client/exthook`), 23.1% (`client`), 13.7% (`cmd/server`)
430+
- **Overall Coverage**: **37.9%** across main-module packages (regenerate with `go test -coverprofile=mergedcoverage ./...` then `go tool cover -func=mergedcoverage`; on PowerShell avoid `-coverprofile=*.out`--see note above)
430431
- **Lines of code (approx.)**: non-test `.go` lines per package directory, same totals as the **Current Coverage Status** table (e.g. `server` 7215, `client` 5555); re-count with:
431432
`python -c "import os; ..."` walking the tree and skipping `*_test.go`, or equivalent `find` + `wc -l`.
432433
- **Execution Time**: on the order of a few seconds for `go test ./...` on a typical dev machine

internal/doctor/doctor_test.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,20 @@ func TestParseLatestReleaseTag(t *testing.T) {
4646
func TestBuildEnvLines_orderAndMask(t *testing.T) {
4747
t.Parallel()
4848
old := osEnviron
49-
t.Cleanup(func() { osEnviron = old })
49+
environMu.Lock()
5050
osEnviron = func() []string {
5151
return []string{
5252
"MARCHAT_PORT=9090",
5353
"MARCHAT_ADMIN_KEY=topsecret",
5454
"MARCHAT_EXTRA_CUSTOM=bar",
5555
}
5656
}
57+
environMu.Unlock()
58+
t.Cleanup(func() {
59+
environMu.Lock()
60+
osEnviron = old
61+
environMu.Unlock()
62+
})
5763
lines := buildEnvLines("server")
5864
foundPort := false
5965
foundExtra := false
@@ -185,7 +191,7 @@ func TestBuildEnvLines_clientIncludesHookVars(t *testing.T) {
185191
func TestBuildEnvLines_serverOmitsClientHookEnvEvenWhenSet(t *testing.T) {
186192
t.Parallel()
187193
old := osEnviron
188-
t.Cleanup(func() { osEnviron = old })
194+
environMu.Lock()
189195
osEnviron = func() []string {
190196
return []string{
191197
"MARCHAT_PORT=8080",
@@ -194,6 +200,12 @@ func TestBuildEnvLines_serverOmitsClientHookEnvEvenWhenSet(t *testing.T) {
194200
"MARCHAT_HOOK_LOG=C:\\Users\\x\\hook.log",
195201
}
196202
}
203+
environMu.Unlock()
204+
t.Cleanup(func() {
205+
environMu.Lock()
206+
osEnviron = old
207+
environMu.Unlock()
208+
})
197209
for _, e := range buildEnvLines("server") {
198210
switch e.Key {
199211
case "MARCHAT_CLIENT_HOOK_RECEIVE", "MARCHAT_CLIENT_HOOK_SEND", "MARCHAT_HOOK_LOG":

internal/doctor/env.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,26 @@ import (
44
"os"
55
"sort"
66
"strings"
7+
"sync"
78
)
89

9-
// osEnviron is the source of environment pairs; tests may replace it.
10-
var osEnviron = os.Environ
10+
// osEnviron is the source of environment pairs; tests may replace it under environMu.
11+
var (
12+
environMu sync.RWMutex
13+
osEnviron = os.Environ
14+
)
1115

1216
type envLine struct {
1317
Key string `json:"key"`
1418
Display string `json:"display"`
1519
}
1620

1721
func collectMarchatEnviron() map[string]string {
22+
environMu.RLock()
23+
fn := osEnviron
24+
environMu.RUnlock()
1825
out := make(map[string]string)
19-
for _, e := range osEnviron() {
26+
for _, e := range fn() {
2027
key, val, ok := strings.Cut(e, "=")
2128
if !ok {
2229
continue

0 commit comments

Comments
 (0)