Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/config-never-blocks-ci.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@flakemetry/contracts': patch
'@flakemetry/cli': patch
---

Stop a broken `flakemetry.yml` from failing a build, and make the tracker's environment settings work.

`flakemetry run` resolved the config before spawning the wrapped command, so an invalid config file raised and the test suite never ran — the wrapper exists precisely so that Flakemetry cannot fail a build. It now warns and carries on. Config errors also arrive as a diagnosis rather than a Node stack trace.

`FLAKEMETRY_TRACKER_ENABLED`, `FLAKEMETRY_TRACKER_AFTER_DAYS` and `FLAKEMETRY_TRACKER_RECOVERY_DAYS` were documented, passed through both compose files, and read by nothing. They now reach the policy layer like every other setting.
41 changes: 41 additions & 0 deletions packages/cli/src/__tests__/config-resilience.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { mkdtempSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'

import { describe, expect, it } from 'vitest'

import { resolveConfig, tryResolveConfig } from '../config-loader'

const withConfig = (body: string): string => {
const dir = mkdtempSync(join(tmpdir(), 'fm-cfg-'))
writeFileSync(join(dir, 'flakemetry.yml'), body)
return dir
}

const BROKEN = `project: acme/web
flaky:
threshold: 1.5
nonsense_key: true
`

describe('tryResolveConfig', () => {
it('reports an invalid config instead of raising', () => {
const attempt = tryResolveConfig(withConfig(BROKEN), {})

expect(attempt.resolved).toBeNull()
expect(attempt.error).toContain('flaky.threshold')
})

it('still resolves a good one', () => {
const attempt = tryResolveConfig(withConfig('project: acme/web\n'), {})

expect(attempt.error).toBeNull()
expect(attempt.resolved?.config.project).toBe('acme/web')
})

it('leaves resolveConfig throwing for callers that need the config', () => {
// `config` and `doctor` exist to tell you the configuration is wrong; they must not
// shrug it off the way the wrapper does.
expect(() => resolveConfig(withConfig(BROKEN), {})).toThrow(/flaky.threshold/)
})
})
20 changes: 20 additions & 0 deletions packages/cli/src/__tests__/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,23 @@ describe('runDoctor', () => {
expect(worstStatus(checks)).toBe('ok')
})
})

describe('config problems never stop the wrapped command', () => {
it('runs the tests and keeps their exit code when the config is broken', async () => {
const notices: string[] = []
const spawner = vi.fn(async () => 0)

const result = await runWrapped({
command,
spawner,
upload: uploaded,
fileExists: () => true,
onNotice: (message) => notices.push(message),
})

// The wrapper exists so Flakemetry can never fail a build. A typo in flakemetry.yml
// preventing a suite from running would be the worst possible way to break that.
expect(spawner).toHaveBeenCalled()
expect(result.exitCode).toBe(0)
})
})
24 changes: 23 additions & 1 deletion packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,26 @@
#!/usr/bin/env node
import { ConfigValidationError } from '@flakemetry/contracts'

import { buildProgram } from './index'

buildProgram(process.cwd(), process.env).parse()
/**
* A configuration mistake is a diagnosis, not a crash. Without this a typo in
* flakemetry.yml reaches the user as a Node stack trace naming a bundled chunk, which
* buries the one part that is actually useful — the list of what is wrong with the file.
*/
const report = (error: unknown): never => {
if (error instanceof ConfigValidationError) {
process.stderr.write(`flakemetry: ${error.message}\n`)
process.exit(1)
}
throw error
}

process.on('uncaughtException', report)
process.on('unhandledRejection', report)

try {
buildProgram(process.cwd(), process.env).parse()
} catch (error) {
report(error)
}
10 changes: 9 additions & 1 deletion packages/cli/src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,21 @@ export const runCommand: CommandModule = {
return
}

// Resolved without raising. A typo in flakemetry.yml must not be the reason a
// test suite never runs: the wrapper exists so that Flakemetry can never fail a
// build, and refusing to start the command would be the worst way to break that.
const attempt = context.tryResolveConfig()
if (attempt.error) {
process.stderr.write(`flakemetry: ignoring the config — ${attempt.error}\n`)
}

const result = await runWrapped({
command,
file: options.file,
endpoint:
options.endpoint ??
context.env.FLAKEMETRY_ENDPOINT ??
context.resolveConfig().config.endpoint,
attempt.resolved?.config.endpoint,
token: options.token ?? context.token,
onNotice: (message) => process.stderr.write(`${message}\n`),
})
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/config-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,28 @@ export const resolveConfig = (
return { config, configPath }
}

export interface ConfigAttempt {
resolved: ResolvedConfig | null
error: string | null
}

/**
* Config resolution that reports a problem instead of raising one. `flakemetry run` wraps a
* test suite, and a typo in flakemetry.yml must not be the reason a suite never runs — the
* whole point of the wrapper is that Flakemetry cannot fail a build. Commands that genuinely
* need the config still check `error` and refuse; the wrapper carries on without it.
*/
export const tryResolveConfig = (
cwd: string,
env: Record<string, string | undefined>,
): ConfigAttempt => {
try {
return { resolved: resolveConfig(cwd, env), error: null }
} catch (error) {
return { resolved: null, error: error instanceof Error ? error.message : String(error) }
}
}

export const resolveToken = (env: Record<string, string | undefined>): string | null =>
env.FLAKEMETRY_TOKEN ?? null

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { junitCommand } from './commands/junit'
import { quarantineCommand } from './commands/quarantine'
import { runCommand } from './commands/run'
import { uploadCommand } from './commands/upload'
import { resolveConfig, resolveToken } from './config-loader'
import { resolveConfig, resolveToken, tryResolveConfig } from './config-loader'
import type { CommandContext } from './registry'
import { CommandRegistry } from './registry'

Expand Down Expand Up @@ -47,6 +47,7 @@ export const buildProgram = (
cwd,
env,
resolveConfig: () => resolveConfig(cwd, env),
tryResolveConfig: () => tryResolveConfig(cwd, env),
token: resolveToken(env),
}
registry.applyTo(program, context)
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/registry.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import type { Command } from 'commander'

import type { ResolvedConfig } from './config-loader'
import type { ConfigAttempt, ResolvedConfig } from './config-loader'

export interface CommandContext {
cwd: string
env: Record<string, string | undefined>
resolveConfig: () => ResolvedConfig
tryResolveConfig: () => ConfigAttempt
token: string | null
}

Expand Down
51 changes: 51 additions & 0 deletions packages/contracts/src/__tests__/policy-env.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { describe, expect, it } from 'vitest'

import { POLICY_FIELDS, projectPolicyEnvOverrides, resolveProjectPolicy } from '../policy'

/**
* Every field the environment tier claims to control must actually reach it. The tracker
* settings were in POLICY_FIELDS, in the compose files, in .env.example and in the
* configuration reference — and read by nothing, so an operator setting them saw the
* dashboard report "default" and the tracker stay off.
*/
const ENV_FOR_FIELD: Readonly<Record<string, [string, string]>> = {
flakyThreshold: ['FLAKEMETRY_FLAKY_THRESHOLD', '0.5'],
minSamples: ['FLAKEMETRY_FLAKY_MIN_SAMPLES', '3'],
quarantineEnabled: ['FLAKEMETRY_QUARANTINE_ENABLED', 'true'],
quarantineCooldownRuns: ['FLAKEMETRY_QUARANTINE_COOLDOWN_RUNS', '7'],
aiRcaEnabled: ['FLAKEMETRY_AI_RCA', 'true'],
ciMinuteCost: ['FLAKEMETRY_CI_MINUTE_COST', '0.02'],
developerHourCost: ['FLAKEMETRY_DEVELOPER_HOUR_COST', '80'],
investigationMinutes: ['FLAKEMETRY_INVESTIGATION_MINUTES', '25'],
trackerEnabled: ['FLAKEMETRY_TRACKER_ENABLED', 'true'],
trackerAfterDays: ['FLAKEMETRY_TRACKER_AFTER_DAYS', '2'],
trackerRecoveryDays: ['FLAKEMETRY_TRACKER_RECOVERY_DAYS', '9'],
}

describe('projectPolicyEnvOverrides', () => {
it('reads every field the effective policy exposes', () => {
const resolved = resolveProjectPolicy({})
const exposed = Object.keys(resolved)

const unreachable = exposed.filter((field) => !(field in ENV_FOR_FIELD))
expect(
unreachable,
'these appear in the effective policy but no environment variable reaches them',
).toEqual([])
})

it.each(Object.entries(ENV_FOR_FIELD))('%s comes through as an env override', (field, pair) => {
const [name, value] = pair
const overrides = projectPolicyEnvOverrides({ [name]: value })

expect(overrides[field as keyof typeof overrides]).toBeDefined()
const resolved = resolveProjectPolicy({ env: overrides })
expect(resolved[field as keyof typeof resolved].source).toBe('env')
})

it('is anchored to the field list, so a new policy field cannot skip this', () => {
// Guard the guard: retention days are policy fields without an effective-policy entry,
// so the count is checked rather than assumed equal.
expect(POLICY_FIELDS.length).toBeGreaterThanOrEqual(Object.keys(ENV_FOR_FIELD).length)
})
})
13 changes: 13 additions & 0 deletions packages/contracts/src/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,18 @@ export const projectPolicyEnvOverrides = (
env.FLAKEMETRY_INVESTIGATION_MINUTES !== ''
)
overrides.investigationMinutes = Number(env.FLAKEMETRY_INVESTIGATION_MINUTES)
// The tracker fields were in POLICY_FIELDS, in both compose files, in .env.example and
// in the configuration reference, and read by nothing — so setting them did nothing and
// the dashboard reported the source as "default". policy-env.test.ts now fails if any
// effective-policy field loses its environment tier again.
if (env.FLAKEMETRY_TRACKER_ENABLED !== undefined && env.FLAKEMETRY_TRACKER_ENABLED !== '')
overrides.trackerEnabled = parseBoolean(env.FLAKEMETRY_TRACKER_ENABLED)
if (env.FLAKEMETRY_TRACKER_AFTER_DAYS !== undefined && env.FLAKEMETRY_TRACKER_AFTER_DAYS !== '')
overrides.trackerAfterDays = Number(env.FLAKEMETRY_TRACKER_AFTER_DAYS)
if (
env.FLAKEMETRY_TRACKER_RECOVERY_DAYS !== undefined &&
env.FLAKEMETRY_TRACKER_RECOVERY_DAYS !== ''
)
overrides.trackerRecoveryDays = Number(env.FLAKEMETRY_TRACKER_RECOVERY_DAYS)
return overrides
}
Loading