fix: stop verify, reports and briefs from claiming more than pipe checked #1
6 changed files with 124 additions and 6 deletions
fix(cli): reject a missing --dir and report filesystem errors cleanly
With several --dir values, one that did not exist was skipped silently: the no-files guard only fires when every directory is empty, so `--dir lib --dir tset` scanned lib and reported as though it had covered what was asked for. A --dir that does not exist is now an error naming it. A directory that could not be listed, or a baseline that could not be read or written, escaped as an uncaught FileSystemException: exit 255 and a stack trace. Those paths now raise a PipeException with the OS reason (exit 2), and bin/pipe.dart catches any remaining FileSystemException the same way. The exit codes are documented in bin/pipe.dart: 0 success, 1 a gate failed, 2 pipe could not do what was asked, 64 usage. rule_scope_claim_test widened the fixture with a test/ directory it does not have, relying on exactly the silent skip; it now builds a project that has one.
commit
ae412a7964
|
|
@ -4,6 +4,14 @@ import 'package:args/command_runner.dart';
|
||||||
import 'package:pipe/src/cli.dart';
|
import 'package:pipe/src/cli.dart';
|
||||||
import 'package:pipe/src/errors.dart';
|
import 'package:pipe/src/errors.dart';
|
||||||
|
|
||||||
|
/// Exit codes:
|
||||||
|
///
|
||||||
|
/// - 0: success.
|
||||||
|
/// - 1: a gate failed: `check --fail-on`, `verify --expect-resolved`,
|
||||||
|
/// `recall --strict`, or `guide --yams` could not persist.
|
||||||
|
/// - 2: pipe could not do what was asked (a [PipeException], or a filesystem
|
||||||
|
/// error such as an unreadable or unwritable path).
|
||||||
|
/// - 64: invalid command-line usage.
|
||||||
Future<void> main(List<String> arguments) async {
|
Future<void> main(List<String> arguments) async {
|
||||||
final runner = PipeRunner();
|
final runner = PipeRunner();
|
||||||
try {
|
try {
|
||||||
|
|
@ -11,6 +19,15 @@ Future<void> main(List<String> arguments) async {
|
||||||
} on PipeException catch (error) {
|
} on PipeException catch (error) {
|
||||||
stderr.writeln(error);
|
stderr.writeln(error);
|
||||||
exitCode = 2;
|
exitCode = 2;
|
||||||
|
} on FileSystemException catch (error) {
|
||||||
|
// Any filesystem failure not already turned into a PipeException still gets
|
||||||
|
// a one-line message and a documented exit code, not a stack trace and 255.
|
||||||
|
stderr.writeln(
|
||||||
|
'pipe: ${error.message}'
|
||||||
|
'${error.path == null ? '' : ': ${error.path}'}'
|
||||||
|
'${error.osError == null ? '' : ' (${error.osError!.message})'}',
|
||||||
|
);
|
||||||
|
exitCode = 2;
|
||||||
} on UsageException catch (error) {
|
} on UsageException catch (error) {
|
||||||
stderr.writeln(error.message);
|
stderr.writeln(error.message);
|
||||||
stderr.writeln(error.usage);
|
stderr.writeln(error.usage);
|
||||||
|
|
|
||||||
|
|
@ -93,6 +93,18 @@ int readPositiveInt(ArgResults results, String name, {required int fallback}) {
|
||||||
return value;
|
return value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Read [path], turning a filesystem failure into a clean [PipeException]
|
||||||
|
/// (exit 2) instead of an uncaught exception (exit 255 and a stack trace).
|
||||||
|
String _readText(String path, String what) {
|
||||||
|
try {
|
||||||
|
return File(path).readAsStringSync();
|
||||||
|
} on FileSystemException catch (error) {
|
||||||
|
throw PipeException(
|
||||||
|
'cannot read $what $path: ${error.osError?.message ?? error.message}',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Set<String>? _readBaseline(ArgResults results) {
|
Set<String>? _readBaseline(ArgResults results) {
|
||||||
final path = results['baseline'] as String?;
|
final path = results['baseline'] as String?;
|
||||||
if (path == null) return null;
|
if (path == null) return null;
|
||||||
|
|
@ -103,13 +115,21 @@ Set<String>? _readBaseline(ArgResults results) {
|
||||||
remedy: 'create one with --write-baseline $path',
|
remedy: 'create one with --write-baseline $path',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
return decodeBaseline(file.readAsStringSync());
|
return decodeBaseline(_readText(path, 'baseline'));
|
||||||
}
|
}
|
||||||
|
|
||||||
void _writeBaseline(ArgResults results, String root, List<Finding> findings) {
|
void _writeBaseline(ArgResults results, String root, List<Finding> findings) {
|
||||||
final path = results['write-baseline'] as String?;
|
final path = results['write-baseline'] as String?;
|
||||||
if (path == null) return;
|
if (path == null) return;
|
||||||
File(path).writeAsStringSync(encodeBaseline(root, findings));
|
try {
|
||||||
|
File(path).writeAsStringSync(encodeBaseline(root, findings));
|
||||||
|
} on FileSystemException catch (error) {
|
||||||
|
throw PipeException(
|
||||||
|
'cannot write baseline $path: '
|
||||||
|
'${error.osError?.message ?? error.message}',
|
||||||
|
remedy: 'check that the directory exists and is writable',
|
||||||
|
);
|
||||||
|
}
|
||||||
// Status goes to stderr: after `--format json` stdout is the JSON document,
|
// Status goes to stderr: after `--format json` stdout is the JSON document,
|
||||||
// and a trailing status line made it unparseable.
|
// and a trailing status line made it unparseable.
|
||||||
stderr.writeln(
|
stderr.writeln(
|
||||||
|
|
@ -511,7 +531,9 @@ class VerifyCommand extends Command<int> {
|
||||||
if (!baselineFile.existsSync()) {
|
if (!baselineFile.existsSync()) {
|
||||||
throw PipeException('baseline file not found: $baselinePath');
|
throw PipeException('baseline file not found: $baselinePath');
|
||||||
}
|
}
|
||||||
final entries = decodeBaselineEntries(baselineFile.readAsStringSync());
|
final entries = decodeBaselineEntries(
|
||||||
|
_readText(baselinePath, 'baseline'),
|
||||||
|
);
|
||||||
final selected = _readRules(results);
|
final selected = _readRules(results);
|
||||||
|
|
||||||
final project = await loadProject(
|
final project = await loadProject(
|
||||||
|
|
|
||||||
|
|
@ -106,8 +106,27 @@ Future<Project> loadProject(
|
||||||
final files = <String>[];
|
final files = <String>[];
|
||||||
for (final directory in directories) {
|
for (final directory in directories) {
|
||||||
final dir = Directory(p.join(root, directory));
|
final dir = Directory(p.join(root, directory));
|
||||||
if (!dir.existsSync()) continue;
|
// A directory that does not exist is a mistake, not an empty scope. With
|
||||||
for (final entity in dir.listSync(recursive: true, followLinks: false)) {
|
// one --dir the no-files guard below caught it, but with several a typo in
|
||||||
|
// one was skipped silently and the scan reported on the rest as if it were
|
||||||
|
// what was asked for.
|
||||||
|
if (!dir.existsSync()) {
|
||||||
|
throw PipeException(
|
||||||
|
'--dir $directory does not exist under $root',
|
||||||
|
remedy: 'check the spelling; --dir is relative to the project root',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
final List<FileSystemEntity> entities;
|
||||||
|
try {
|
||||||
|
entities = dir.listSync(recursive: true, followLinks: false);
|
||||||
|
} on FileSystemException catch (error) {
|
||||||
|
throw PipeException(
|
||||||
|
'cannot list --dir $directory: '
|
||||||
|
'${error.osError?.message ?? error.message} (${error.path ?? dir.path})',
|
||||||
|
remedy: 'check that the directory is readable',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
for (final entity in entities) {
|
||||||
if (entity is! File) continue;
|
if (entity is! File) continue;
|
||||||
if (!entity.path.endsWith('.dart')) continue;
|
if (!entity.path.endsWith('.dart')) continue;
|
||||||
if (_isHidden(entity.path, root)) continue;
|
if (_isHidden(entity.path, root)) continue;
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,5 @@
|
||||||
|
import 'dart:io';
|
||||||
|
|
||||||
import 'package:args/args.dart';
|
import 'package:args/args.dart';
|
||||||
import 'package:pipe/pipe.dart';
|
import 'package:pipe/pipe.dart';
|
||||||
import 'package:pipe/src/cli.dart';
|
import 'package:pipe/src/cli.dart';
|
||||||
|
|
@ -166,4 +168,27 @@ void main() {
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('an unwritable baseline path is a clean error, not a crash', () async {
|
||||||
|
// writeAsStringSync threw an uncaught FileSystemException: exit 255 and a
|
||||||
|
// stack trace. It is now a PipeException, which exits 2 with one line.
|
||||||
|
final missing = Directory.systemTemp.createTempSync('pipe-nowrite');
|
||||||
|
await expectLater(
|
||||||
|
PipeRunner().run([
|
||||||
|
'check',
|
||||||
|
'test/fixtures/stuck_loading',
|
||||||
|
'--write-baseline',
|
||||||
|
'${missing.path}/no/such/dir/baseline.json',
|
||||||
|
'-c',
|
||||||
|
'1',
|
||||||
|
]),
|
||||||
|
throwsA(
|
||||||
|
isA<PipeException>().having(
|
||||||
|
(error) => error.toString(),
|
||||||
|
'message',
|
||||||
|
contains('cannot write baseline'),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -49,6 +49,25 @@ void main() {
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
test('one mistyped --dir among several is an error, not skipped', () async {
|
||||||
|
// With a single --dir the no-files guard caught a typo, but with several a
|
||||||
|
// missing one was skipped silently and the scan reported on the others as
|
||||||
|
// though it covered what was asked for.
|
||||||
|
await expectLater(
|
||||||
|
loadProject(
|
||||||
|
'test/fixtures/stuck_loading',
|
||||||
|
directories: const ['lib', 'tset'],
|
||||||
|
),
|
||||||
|
throwsA(
|
||||||
|
isA<PipeException>().having(
|
||||||
|
(error) => error.toString(),
|
||||||
|
'message',
|
||||||
|
contains('--dir tset does not exist'),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test('a real project still loads', () async {
|
test('a real project still loads', () async {
|
||||||
final project = await loadProject('test/fixtures/stuck_loading');
|
final project = await loadProject('test/fixtures/stuck_loading');
|
||||||
expect(project.dartFiles, isNotEmpty);
|
expect(project.dartFiles, isNotEmpty);
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,5 @@
|
||||||
import 'dart:convert';
|
import 'dart:convert';
|
||||||
|
import 'dart:io';
|
||||||
|
|
||||||
import 'package:pipe/pipe.dart';
|
import 'package:pipe/pipe.dart';
|
||||||
import 'package:test/test.dart';
|
import 'package:test/test.dart';
|
||||||
|
|
@ -47,8 +48,23 @@ void main() {
|
||||||
final defaulted = await loadProject('test/fixtures/stuck_loading');
|
final defaulted = await loadProject('test/fixtures/stuck_loading');
|
||||||
expect(defaulted.directories, const ['lib']);
|
expect(defaulted.directories, const ['lib']);
|
||||||
|
|
||||||
|
// The fixture has no test/ directory, and a --dir that does not exist is
|
||||||
|
// now an error rather than skipped, so widen a copy that has one.
|
||||||
|
final temp = Directory.systemTemp.createTempSync('pipe-widened');
|
||||||
|
Directory('${temp.path}/.dart_tool').createSync();
|
||||||
|
File('${temp.path}/.dart_tool/package_config.json').writeAsStringSync(
|
||||||
|
File(
|
||||||
|
'test/fixtures/stuck_loading/.dart_tool/package_config.json',
|
||||||
|
).readAsStringSync(),
|
||||||
|
);
|
||||||
|
File('${temp.path}/lib/a.dart')
|
||||||
|
..createSync(recursive: true)
|
||||||
|
..writeAsStringSync('class A {}\n');
|
||||||
|
File('${temp.path}/test/a_test.dart')
|
||||||
|
..createSync(recursive: true)
|
||||||
|
..writeAsStringSync('void main() {}\n');
|
||||||
final widened = await loadProject(
|
final widened = await loadProject(
|
||||||
'test/fixtures/stuck_loading',
|
temp.path,
|
||||||
directories: const ['lib', 'test'],
|
directories: const ['lib', 'test'],
|
||||||
);
|
);
|
||||||
expect(widened.directories, const ['lib', 'test']);
|
expect(widened.directories, const ['lib', 'test']);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue