fix(cli): close the remaining rough edges in paths, recall, yams and baselines #4
6 changed files with 200 additions and 13 deletions
|
|
@ -94,7 +94,9 @@ with `--yams-name <name>` persists the full markdown report (not the
|
|||
budget-trimmed brief) to the yams corpus, tagged with the analysed commit and a
|
||||
generation timestamp;
|
||||
`recall --manifest <path> --root <dir>` locate the labelled-bug manifest and the
|
||||
directory holding the repos it names, and `recall --strict` exits non-zero when a
|
||||
directory holding the repos it names (without `--manifest`, `recall/manifest.json`
|
||||
in the working directory, else the one in the pipe checkout the binary was built
|
||||
from), and `recall --strict` exits non-zero when a
|
||||
case it expects to still find is missed. Every numeric flag rejects a value that
|
||||
is not a positive integer rather than falling back to its default.
|
||||
|
||||
|
|
@ -115,7 +117,9 @@ Output contract:
|
|||
`verify --expect-resolved`, `recall --strict`, `index --fail-on-error`, or
|
||||
`guide --yams` could not persist); 2 pipe could not do what was asked (a bad
|
||||
path or `--dir`, an unreadable project, a baseline that cannot be read or
|
||||
written); 64 invalid usage.
|
||||
written, or an internal error - rerun with `PIPE_DEBUG=1` for its stack
|
||||
trace); 64 invalid usage, including more than one project path.
|
||||
`guide --yams --write-baseline` still writes the baseline when yams fails.
|
||||
|
||||
`pipe` finds the right SDK itself: it reads `flutterRoot` from the target's
|
||||
`.dart_tool/package_config.json`, then `FLUTTER_ROOT`, then `flutter` on `PATH`.
|
||||
|
|
|
|||
|
|
@ -33,5 +33,16 @@ Future<void> main(List<String> arguments) async {
|
|||
stderr.writeln(error.message);
|
||||
stderr.writeln(error.usage);
|
||||
exitCode = 64;
|
||||
} catch (error, stack) {
|
||||
// Anything else is a pipe bug. Say so in one line with the documented
|
||||
// "could not do what was asked" code instead of a raw trace and exit 255;
|
||||
// PIPE_DEBUG=1 restores the trace for a bug report.
|
||||
stderr.writeln('pipe: internal error: $error');
|
||||
if (Platform.environment['PIPE_DEBUG'] == '1') {
|
||||
stderr.writeln(stack);
|
||||
} else {
|
||||
stderr.writeln('pipe: rerun with PIPE_DEBUG=1 for a stack trace');
|
||||
}
|
||||
exitCode = 2;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -122,7 +122,11 @@ void _writeBaseline(ArgResults results, String root, List<Finding> findings) {
|
|||
final path = results['write-baseline'] as String?;
|
||||
if (path == null) return;
|
||||
try {
|
||||
File(path).writeAsStringSync(encodeBaseline(root, findings));
|
||||
// Record the project root relative to the baseline file, so a committed
|
||||
// baseline does not carry one machine's absolute path (and does not churn
|
||||
// when another checkout rewrites it). Matching never reads `root`.
|
||||
final relativeRoot = p.relative(root, from: p.dirname(p.absolute(path)));
|
||||
File(path).writeAsStringSync(encodeBaseline(relativeRoot, findings));
|
||||
} on FileSystemException catch (error) {
|
||||
throw PipeException(
|
||||
'cannot write baseline $path: '
|
||||
|
|
@ -169,8 +173,50 @@ Set<String> _readRules(ArgResults results) {
|
|||
return selected;
|
||||
}
|
||||
|
||||
String _target(ArgResults results) =>
|
||||
results.rest.isEmpty ? '.' : results.rest.first;
|
||||
/// The one project path a command analyses, `.` when none is given.
|
||||
///
|
||||
/// Extra positionals used to be dropped silently, so `pipe check a b` analysed
|
||||
/// only `a` and reported as if both had been checked.
|
||||
String _target(ArgResults results) {
|
||||
final rest = results.rest;
|
||||
if (rest.length > 1) {
|
||||
throw UsageException(
|
||||
'expected one project path, got ${rest.length}: ${rest.join(' ')}',
|
||||
'Run "pipe help <command>" for usage; run pipe once per project.',
|
||||
);
|
||||
}
|
||||
return rest.isEmpty ? '.' : rest.first;
|
||||
}
|
||||
|
||||
/// Where `recall` looks for its manifest when `--manifest` is not given.
|
||||
///
|
||||
/// The default used to be `recall/manifest.json` relative to the working
|
||||
/// directory, so `pipe recall` only worked from inside the pipe repo. Prefer that
|
||||
/// path when it exists, otherwise search upward from the running script and
|
||||
/// executable (the source checkout for `dart run`, the build bundle for the AOT
|
||||
/// binary) for the repo's `recall/manifest.json`.
|
||||
String defaultRecallManifest({String? cwd, List<String>? searchFrom}) {
|
||||
const relative = 'recall/manifest.json';
|
||||
final local = p.join(cwd ?? Directory.current.path, relative);
|
||||
if (File(local).existsSync()) return local;
|
||||
final starts =
|
||||
searchFrom ??
|
||||
[
|
||||
if (Platform.script.scheme == 'file') Platform.script.toFilePath(),
|
||||
Platform.resolvedExecutable,
|
||||
];
|
||||
for (final start in starts) {
|
||||
var dir = p.dirname(p.absolute(start));
|
||||
while (true) {
|
||||
final candidate = p.join(dir, relative);
|
||||
if (File(candidate).existsSync()) return candidate;
|
||||
final parent = p.dirname(dir);
|
||||
if (parent == dir) break;
|
||||
dir = parent;
|
||||
}
|
||||
}
|
||||
return local;
|
||||
}
|
||||
|
||||
String _encode(Map<String, Object?> json) =>
|
||||
const JsonEncoder.withIndent(' ').convert(json);
|
||||
|
|
@ -424,8 +470,9 @@ class RecallCommand extends Command<int> {
|
|||
argParser
|
||||
..addOption(
|
||||
'manifest',
|
||||
defaultsTo: 'recall/manifest.json',
|
||||
help: 'Manifest of labelled known bugs.',
|
||||
help:
|
||||
'Manifest of labelled known bugs (default: recall/manifest.json '
|
||||
'in the working directory, else in the pipe checkout).',
|
||||
)
|
||||
..addOption(
|
||||
'root',
|
||||
|
|
@ -466,7 +513,7 @@ class RecallCommand extends Command<int> {
|
|||
final results = argResults!;
|
||||
final root = (results['root'] as String?) ?? Directory.current.path;
|
||||
final report = await runRecall(
|
||||
manifestPath: results['manifest'] as String,
|
||||
manifestPath: (results['manifest'] as String?) ?? defaultRecallManifest(),
|
||||
root: root,
|
||||
sdkOverride: results['sdk'] as String?,
|
||||
concurrency: _readConcurrency(results),
|
||||
|
|
@ -784,6 +831,9 @@ class GuideCommand extends Command<int> {
|
|||
collection: 'pipe',
|
||||
);
|
||||
if (error != null) {
|
||||
// Write the baseline anyway: a yams outage must not also lose the
|
||||
// accepted findings the caller asked to record.
|
||||
_writeBaseline(results, project.root, analysis.report.findings);
|
||||
stderr.writeln('pipe: could not persist to yams: $error');
|
||||
return 1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -743,15 +743,20 @@ Future<String?> emitToYams({
|
|||
if (collection != null) arguments.addAll(['--collection', collection]);
|
||||
|
||||
final process = await Process.start(executable, arguments);
|
||||
// Drain both pipes concurrently. Reading only stderr let a chatty `yams add`
|
||||
// fill its stdout buffer and block forever.
|
||||
final stdoutDone = process.stdout.drain<void>();
|
||||
final stderrText = process.stderr
|
||||
.transform(const SystemEncoding().decoder)
|
||||
.join();
|
||||
process.stdin.write(content);
|
||||
await process.stdin.flush();
|
||||
await process.stdin.close();
|
||||
final stderrText = await process.stderr
|
||||
.transform(const SystemEncoding().decoder)
|
||||
.join();
|
||||
await stdoutDone;
|
||||
final errorText = await stderrText;
|
||||
final exitCode = await process.exitCode;
|
||||
if (exitCode != 0) {
|
||||
return 'yams add failed (exit $exitCode): ${stderrText.trim()}';
|
||||
return 'yams add failed (exit $exitCode): ${errorText.trim()}';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -207,7 +207,7 @@ List<RecallCase> loadRecallCases(String manifestPath) {
|
|||
if (!file.existsSync()) {
|
||||
throw PipeException(
|
||||
'recall manifest not found: $manifestPath',
|
||||
remedy: 'pass --manifest <path> or run from the pipe repo root',
|
||||
remedy: 'pass --manifest <path>',
|
||||
);
|
||||
}
|
||||
final Object? decoded;
|
||||
|
|
|
|||
117
test/cli_edges_test.dart
Normal file
117
test/cli_edges_test.dart
Normal file
|
|
@ -0,0 +1,117 @@
|
|||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:args/command_runner.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:pipe/src/cli.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
void main() {
|
||||
test(
|
||||
'a second project path is a usage error, not silently dropped',
|
||||
() async {
|
||||
// `pipe check a b` used to analyse only `a` and report as if both ran.
|
||||
await expectLater(
|
||||
PipeRunner().run(['check', 'test/fixtures/stuck_loading', 'lib']),
|
||||
throwsA(
|
||||
isA<UsageException>().having(
|
||||
(e) => e.message,
|
||||
'message',
|
||||
contains('expected one project path, got 2'),
|
||||
),
|
||||
),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
test('recall finds the pipe manifest from outside the repo', () {
|
||||
// The default used to be cwd-relative, so `pipe recall` failed anywhere
|
||||
// but the pipe checkout.
|
||||
final elsewhere = Directory.systemTemp.createTempSync('pipe-recall-cwd');
|
||||
final script = p.join(Directory.current.path, 'bin', 'pipe.dart');
|
||||
final found = defaultRecallManifest(
|
||||
cwd: elsewhere.path,
|
||||
searchFrom: [script],
|
||||
);
|
||||
expect(
|
||||
p.equals(found, p.join(Directory.current.path, 'recall/manifest.json')),
|
||||
isTrue,
|
||||
reason: found,
|
||||
);
|
||||
});
|
||||
|
||||
test('recall prefers a manifest in the working directory', () {
|
||||
final temp = Directory.systemTemp.createTempSync('pipe-recall-local');
|
||||
final local = File(p.join(temp.path, 'recall', 'manifest.json'))
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync('[]');
|
||||
expect(
|
||||
defaultRecallManifest(cwd: temp.path, searchFrom: const []),
|
||||
local.path,
|
||||
);
|
||||
});
|
||||
|
||||
test('guide --yams writes the baseline even when yams fails, and does not '
|
||||
'hang on a chatty yams', () async {
|
||||
// A fake yams that floods stdout (more than a pipe buffer) and then fails.
|
||||
// Before: pipe read only stderr, so the child blocked on a full stdout and
|
||||
// the run hung; and the yams failure returned before --write-baseline ran.
|
||||
final temp = Directory.systemTemp.createTempSync('pipe-fake-yams');
|
||||
final fake = File(p.join(temp.path, 'yams'))
|
||||
..writeAsStringSync(
|
||||
'#!/bin/sh\n'
|
||||
'cat >/dev/null\n'
|
||||
'head -c 1048576 /dev/zero | tr "\\\\0" x\n'
|
||||
'echo "fake yams down" >&2\n'
|
||||
'exit 3\n',
|
||||
);
|
||||
await Process.run('chmod', ['+x', fake.path]);
|
||||
final baseline = p.join(temp.path, 'baseline.json');
|
||||
|
||||
final result = await Process.run(
|
||||
Platform.resolvedExecutable,
|
||||
[
|
||||
'bin/pipe.dart',
|
||||
'guide',
|
||||
'test/fixtures/stuck_loading',
|
||||
'--yams',
|
||||
'--write-baseline',
|
||||
baseline,
|
||||
'-c',
|
||||
'1',
|
||||
],
|
||||
environment: {'PATH': '${temp.path}:/usr/bin:/bin'},
|
||||
).timeout(const Duration(minutes: 2));
|
||||
|
||||
expect(result.exitCode, 1, reason: '${result.stderr}');
|
||||
expect(result.stderr as String, contains('fake yams down'));
|
||||
expect(File(baseline).existsSync(), isTrue);
|
||||
});
|
||||
|
||||
test('a written baseline records its root relative to the file', () async {
|
||||
// It used to store the absolute project path, so a committed baseline
|
||||
// leaked one machine's home directory and churned on every checkout.
|
||||
final temp = Directory.systemTemp.createTempSync('pipe-baseline-root');
|
||||
final baseline = p.join(temp.path, 'baseline.json');
|
||||
final result = await Process.run(Platform.resolvedExecutable, [
|
||||
'bin/pipe.dart',
|
||||
'check',
|
||||
'test/fixtures/stuck_loading',
|
||||
'--write-baseline',
|
||||
baseline,
|
||||
'-c',
|
||||
'1',
|
||||
]);
|
||||
expect(result.exitCode, 0, reason: '${result.stderr}');
|
||||
final root = (jsonDecode(File(baseline).readAsStringSync()) as Map)['root'];
|
||||
expect(p.isAbsolute(root as String), isFalse, reason: root);
|
||||
expect(
|
||||
p.equals(
|
||||
p.normalize(p.join(temp.path, root)),
|
||||
p.absolute('test/fixtures/stuck_loading'),
|
||||
),
|
||||
isTrue,
|
||||
reason: root,
|
||||
);
|
||||
});
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue