From 24e8c5576c19a7e6340083986b27c990a54992cf Mon Sep 17 00:00:00 2001 From: Brendan Ryan Date: Sun, 18 Jan 2026 19:59:32 -0800 Subject: [PATCH 1/2] integration tests --- bin/tempo-lints.ts | 14 +++- scripts/action-integration.test.ts | 111 +++++++++++++++++++++++++++++ scripts/shared.test.ts | 106 +++++++++++++++++++++++++++ scripts/shared.ts | 61 ++++++++++++++-- test-fixtures/rust/mixed.rs | 14 ++++ test-fixtures/rust/valid.rs | 19 +++++ test-fixtures/rust/with-dbg.rs | 12 ++++ test-fixtures/rust/with-unwrap.rs | 14 ++++ 8 files changed, 344 insertions(+), 7 deletions(-) create mode 100644 scripts/action-integration.test.ts create mode 100644 test-fixtures/rust/mixed.rs create mode 100644 test-fixtures/rust/valid.rs create mode 100644 test-fixtures/rust/with-dbg.rs create mode 100644 test-fixtures/rust/with-unwrap.rs diff --git a/bin/tempo-lints.ts b/bin/tempo-lints.ts index cb1cff3..819c8af 100644 --- a/bin/tempo-lints.ts +++ b/bin/tempo-lints.ts @@ -11,6 +11,7 @@ import { generateConfigContent, getRuleDirs, getRuleDirsRelative, + getValidRuleIds, isValidLanguage, LANG, type Language, @@ -47,7 +48,7 @@ function runScan(language: string, scanPath: string, options: ScanOptions): void const excludeRules = options.exclude?.split(',').map((r) => r.trim()) ?? [] - runAstGrep(configPath, scanPath, { + runAstGrep(language, configPath, scanPath, { json: options.json ?? false, fix: options.fix ?? false, githubAction: options.githubAction ?? false, @@ -64,7 +65,12 @@ interface AstGrepOptions { cleanup: () => void } -function runAstGrep(configPath: string, scanPath: string, options: AstGrepOptions): void { +function runAstGrep( + language: Language, + configPath: string, + scanPath: string, + options: AstGrepOptions, +): void { const { cleanup } = options const handleSignal = (signal: 'SIGINT' | 'SIGTERM') => { @@ -108,7 +114,9 @@ function runAstGrep(configPath: string, scanPath: string, options: AstGrepOption cleanup() if (useJson && output) { - const { issues, error } = parseLintIssues(output) + // Get valid rule IDs to filter out non-tempo lint entries + const validRuleIds = getValidRuleIds(language) + const { issues, error } = parseLintIssues(output, validRuleIds) if (error) { warn(error) diff --git a/scripts/action-integration.test.ts b/scripts/action-integration.test.ts new file mode 100644 index 0000000..74beaa3 --- /dev/null +++ b/scripts/action-integration.test.ts @@ -0,0 +1,111 @@ +/** + * Integration tests for the GitHub Action + * Tests the full flow: lint → JSON output → PR comment generation + */ + +import { execSync } from 'node:child_process' +import path from 'node:path' +import { describe, expect, it } from 'vitest' +import { countBySeverity, type LintIssue, PACKAGE_ROOT, parseLintIssues } from './shared.ts' + +const FIXTURES_DIR = path.join(PACKAGE_ROOT, 'test-fixtures/rust') +const CLI_PATH = path.join(PACKAGE_ROOT, 'bin/tempo-lints.ts') + +function runLintsOnFixtures(): LintIssue[] { + try { + const output = execSync(`pnpm exec tsx "${CLI_PATH}" rust "${FIXTURES_DIR}" --json`, { + encoding: 'utf8', + cwd: PACKAGE_ROOT, + stdio: ['pipe', 'pipe', 'pipe'], + }) + + const { issues, error } = parseLintIssues(output) + if (error) { + throw new Error(`Failed to parse lint output: ${error}`) + } + return issues + } catch (err) { + // execSync throws on non-zero exit, but we still want the output + const error = err as { stdout?: string } + if (error.stdout) { + const { issues, error: parseError } = parseLintIssues(error.stdout) + if (!parseError) { + return issues + } + } + throw err + } +} + +describe('Action Integration Tests', () => { + it('should find lint violations in test fixtures', () => { + const issues = runLintsOnFixtures() + expect(issues.length).toBeGreaterThan(0) + }) + + it('should only return tempo lint rule IDs (not other tools)', () => { + const issues = runLintsOnFixtures() + + // All rule IDs should be from tempo lints, not other tools + for (const issue of issues) { + expect(issue.ruleId).not.toMatch(/^clippy::/) + expect(issue.ruleId).not.toBe('unknown') + } + }) + + it('should find dbg! macro violations', () => { + const issues = runLintsOnFixtures() + const dbgIssues = issues.filter((issue) => issue.ruleId === 'no-dbg-macro') + expect(dbgIssues.length).toBeGreaterThan(0) + }) + + it('should find unwrap violations', () => { + const issues = runLintsOnFixtures() + const unwrapIssues = issues.filter((issue) => issue.ruleId === 'no-unwrap-in-lib') + expect(unwrapIssues.length).toBeGreaterThan(0) + }) + + it('should return issues with all required fields', () => { + const issues = runLintsOnFixtures() + + for (const issue of issues) { + expect(issue.ruleId).toBeTruthy() + expect(issue.file).toBeTruthy() + expect(issue.line).toBeGreaterThan(0) + expect(issue.severity).toBeTruthy() + expect(issue.message).toBeTruthy() + } + }) + + it('should correctly count issues by severity', () => { + const issues = runLintsOnFixtures() + const counts = countBySeverity(issues) + + expect(counts.error).toBeGreaterThanOrEqual(0) + expect(counts.warning).toBeGreaterThanOrEqual(0) + expect(counts.hint).toBeGreaterThanOrEqual(0) + expect(counts.error + counts.warning + counts.hint).toBe(issues.length) + }) + + it('should generate valid JSON output', () => { + let output: string + try { + output = execSync(`pnpm exec tsx "${CLI_PATH}" rust "${FIXTURES_DIR}" --json`, { + encoding: 'utf8', + cwd: PACKAGE_ROOT, + stdio: ['pipe', 'pipe', 'pipe'], + }) + } catch (err) { + // Command exits with non-zero when errors are found, but still outputs JSON + const error = err as { stdout?: string } + output = error.stdout || '' + } + + // Should be valid JSON + expect(() => JSON.parse(output)).not.toThrow() + + // Should be an array + const parsed = JSON.parse(output) + expect(Array.isArray(parsed)).toBe(true) + }) +}) diff --git a/scripts/shared.test.ts b/scripts/shared.test.ts index 7b6f439..77c817f 100644 --- a/scripts/shared.test.ts +++ b/scripts/shared.test.ts @@ -8,6 +8,7 @@ import { generateConfigContent, getRuleDirs, getRuleDirsRelative, + getValidRuleIds, isValidLanguage, LANG, type LintIssue, @@ -511,3 +512,108 @@ describe('LANG constants', () => { expect(LANG.ALL).toBe('all') }) }) + +describe('getValidRuleIds', () => { + it('should return rust rule IDs', () => { + const ids = Array.from(getValidRuleIds(LANG.RUST)) + + // Should include known rust rules + expect(ids).toContain('no-dbg-macro') + expect(ids).toContain('no-unwrap-in-lib') + expect(ids).toContain('no-mem-transmute') + expect(ids).toContain('tracing-no-format') + expect(ids).toContain('unsafe-needs-safety-comment') + }) + + it('should return typescript rule IDs', () => { + const ids = Array.from(getValidRuleIds(LANG.TYPESCRIPT)) + + // Should include known typescript rules (if any exist) + expect(ids.length).toBeGreaterThanOrEqual(0) + }) + + it('should return all rule IDs for "all" language', () => { + const ids = Array.from(getValidRuleIds(LANG.ALL)) + + // Should include rules from both languages + expect(ids).toContain('no-dbg-macro') + expect(ids.length).toBeGreaterThan(0) + }) +}) + +describe('parseLintIssues with rule ID filtering', () => { + it('should filter out entries with invalid ruleId', () => { + const mixedOutput = JSON.stringify([ + { + ruleId: 'no-dbg-macro', + file: 'test.rs', + range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } }, + message: 'Remove dbg! macro', + severity: 'error', + }, + { + ruleId: 'clippy::unwrap_used', + file: 'test.rs', + range: { start: { line: 2, column: 1 }, end: { line: 2, column: 10 } }, + message: 'Clippy warning', + severity: 'warning', + }, + { + ruleId: 'unknown-rule', + file: 'test.rs', + range: { start: { line: 3, column: 1 }, end: { line: 3, column: 10 } }, + message: 'Unknown', + severity: 'warning', + }, + ]) + + const validIds = new Set(['no-dbg-macro', 'no-unwrap-in-lib']) + const result = parseLintIssues(mixedOutput, validIds) + + expect(result.error).toBeNull() + expect(result.issues).toHaveLength(1) + expect(result.issues[0]?.ruleId).toBe('no-dbg-macro') + }) + + it('should pass through all entries when no validRuleIds provided', () => { + const output = JSON.stringify([ + { + ruleId: 'no-dbg-macro', + file: 'test.rs', + range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } }, + message: 'Remove dbg! macro', + severity: 'error', + }, + { + ruleId: 'unknown-rule', + file: 'test.rs', + range: { start: { line: 2, column: 1 }, end: { line: 2, column: 10 } }, + message: 'Unknown', + severity: 'warning', + }, + ]) + + const result = parseLintIssues(output) + + expect(result.error).toBeNull() + expect(result.issues).toHaveLength(2) + }) + + it('should filter all entries if none match validRuleIds', () => { + const output = JSON.stringify([ + { + ruleId: 'clippy::unwrap_used', + file: 'test.rs', + range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } }, + message: 'Clippy warning', + severity: 'warning', + }, + ]) + + const validIds = new Set(['no-dbg-macro']) + const result = parseLintIssues(output, validIds) + + expect(result.error).toBeNull() + expect(result.issues).toHaveLength(0) + }) +}) diff --git a/scripts/shared.ts b/scripts/shared.ts index 9339658..9ba2fe2 100644 --- a/scripts/shared.ts +++ b/scripts/shared.ts @@ -75,8 +75,9 @@ function isAstGrepIssueArray(value: unknown): value is AstGrepIssue[] { for (let i = 0; i < samplesToCheck; i++) { const item = value[i] if (typeof item !== 'object' || item === null) return false - // ast-grep always includes file and range - if (!('file' in item || 'ruleId' in item)) { + // Require ruleId field to ensure we only accept lint rule violations + // This filters out parse errors and other non-lint entries + if (!('ruleId' in item)) { return false } } @@ -96,7 +97,10 @@ function normalizeIssue(issue: AstGrepIssue): LintIssue { } } -export function parseLintIssues(input: string): { issues: LintIssue[]; error: string | null } { +export function parseLintIssues( + input: string, + validRuleIds?: Set, +): { issues: LintIssue[]; error: string | null } { const { data, error } = safeParseJSON(input) if (error) { @@ -108,7 +112,24 @@ export function parseLintIssues(input: string): { issues: LintIssue[]; error: st } // Normalize ast-grep issues to our format - const issues = data.map(normalizeIssue) + let issues = data.map(normalizeIssue) + + // Filter by valid rule IDs if provided (to exclude non-tempo lint entries) + if (validRuleIds) { + const filteredOut: LintIssue[] = [] + issues = issues.filter((issue) => { + if (validRuleIds.has(issue.ruleId)) { + return true + } + filteredOut.push(issue) + return false + }) + + // Warn about filtered issues + for (const issue of filteredOut) { + warn(`Filtered out non-tempo lint issue: ${issue.ruleId} in ${issue.file}:${issue.line}`) + } + } return { issues, error: null } } @@ -149,6 +170,38 @@ export function getRuleDirsRelative(language: Language): string[] { return dirs } +/** + * Get all valid rule IDs for a given language by scanning rule directories. + * This is used to filter out non-lint entries from the output. + */ +export function getValidRuleIds( + language: Language, + packageRoot: string = PACKAGE_ROOT, +): Set { + const ruleDirs = getRuleDirs(language, packageRoot) + const ids = new Set() + + for (const dir of ruleDirs) { + if (!fs.existsSync(dir)) { + continue + } + + const files = fs.readdirSync(dir) + for (const file of files) { + if (file.endsWith('.yml')) { + const filePath = path.join(dir, file) + const content = fs.readFileSync(filePath, 'utf8') + const match = content.match(/^id:\s*(.+)$/m) + if (match) { + ids.add(match[1].trim()) + } + } + } + } + + return ids +} + export function generateConfigContent(ruleDirs: string[]): string { return `ruleDirs:\n${ruleDirs.map((d) => ` - ${d}`).join('\n')}\n` } diff --git a/test-fixtures/rust/mixed.rs b/test-fixtures/rust/mixed.rs new file mode 100644 index 0000000..1d73e12 --- /dev/null +++ b/test-fixtures/rust/mixed.rs @@ -0,0 +1,14 @@ +// This file contains multiple types of violations + +pub fn process_data(input: Option) -> String { + let data = input.unwrap(); // no-unwrap-in-lib violation + + dbg!(&data); // no-dbg-macro violation + + data.to_uppercase() +} + +pub fn debug_value(x: i32) { + dbg!(x); // Another dbg! violation + println!("Value: {}", x); +} diff --git a/test-fixtures/rust/valid.rs b/test-fixtures/rust/valid.rs new file mode 100644 index 0000000..6dfc0c6 --- /dev/null +++ b/test-fixtures/rust/valid.rs @@ -0,0 +1,19 @@ +// This file should have no lint violations + +fn add(a: i32, b: i32) -> i32 { + a + b +} + +fn multiply(a: i32, b: i32) -> i32 { + a * b +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_add() { + assert_eq!(add(2, 3), 5); + } +} diff --git a/test-fixtures/rust/with-dbg.rs b/test-fixtures/rust/with-dbg.rs new file mode 100644 index 0000000..4c33e09 --- /dev/null +++ b/test-fixtures/rust/with-dbg.rs @@ -0,0 +1,12 @@ +// This file contains dbg!() macro which should be flagged + +fn calculate(x: i32, y: i32) -> i32 { + let result = x + y; + dbg!(result); // This should be caught by no-dbg-macro rule + result +} + +fn main() { + let value = 42; + dbg!(value); // Another dbg! to catch +} diff --git a/test-fixtures/rust/with-unwrap.rs b/test-fixtures/rust/with-unwrap.rs new file mode 100644 index 0000000..ddc4216 --- /dev/null +++ b/test-fixtures/rust/with-unwrap.rs @@ -0,0 +1,14 @@ +// This file contains .unwrap() calls which should be flagged in library code + +pub fn parse_number(s: &str) -> i32 { + s.parse::().unwrap() // Should be caught by no-unwrap-in-lib +} + +pub fn get_first(vec: Vec) -> T { + vec.into_iter().next().unwrap() // Another unwrap to catch +} + +fn main() { + let result = parse_number("42"); + println!("{}", result); +} From 87b2ea3dd7b40679f5c0114f70c169049e17756f Mon Sep 17 00:00:00 2001 From: Brendan Ryan Date: Sun, 18 Jan 2026 20:04:29 -0800 Subject: [PATCH 2/2] fix patch --- scripts/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/shared.ts b/scripts/shared.ts index 9ba2fe2..86d6c8f 100644 --- a/scripts/shared.ts +++ b/scripts/shared.ts @@ -192,7 +192,7 @@ export function getValidRuleIds( const filePath = path.join(dir, file) const content = fs.readFileSync(filePath, 'utf8') const match = content.match(/^id:\s*(.+)$/m) - if (match) { + if (match?.[1]) { ids.add(match[1].trim()) } }