Skip to content

Commit

Permalink
Fix root suite comparison on Windows (#2038)
Browse files Browse the repository at this point in the history
Fixes #2037

Canonicalize paths when checking whether a stack frame is coming from
the test suite root.

Add a test case which runs with an absolute path argument. On windows,
this situation would have resulted in the `root_` location information
being missing because of a difference in the stack frame paths and the
suite path before canonicalization. This test fails on windows without
the canonicalization.

Make the test utils more flexible to handle the new output differences
across operation systems.
Allow a `Matcher` value for the `path` argument to `suiteJson`. This
optional argument had previously been unused. Use `equalsIgnoreCase`
because on windows there is a difference in the case of the drive
letter.
Allow a `Matcher` value for the `name` argument to `testStartJson`. Take
an `Object` to allow either `Matcher` or `String` for existing uses.
Check the "loading" output line loosely by checking only the start and
end.
  • Loading branch information
natebosch committed Jun 16, 2023
1 parent 02fcb09 commit 3d5afed
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 8 deletions.
2 changes: 2 additions & 0 deletions pkgs/test/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* Drop support for null unsafe Dart, bump SDK constraint to `3.0.0`.
* Make some annotation classes `final`: `OnPlatform`, `Retry`, `Skip`, `Tags`,
`TestOn`, `Timeout`.
* Fix the `root_` fields in the JSON reporter when running a test on Windows
with an absolute path.

## 1.24.3

Expand Down
46 changes: 44 additions & 2 deletions pkgs/test/test/runner/json_reporter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ void main() {
args: ['-p', 'chrome']);
}, tags: ['chrome'], skip: 'https://github.com/dart-lang/test/issues/872');

test('the root suite if applicable', () {
test('the root suite from a relative path', () {
return _expectReport(
'''
customTest('success 1', () {});
Expand Down Expand Up @@ -543,6 +543,46 @@ void main() {
'common.dart': '''
import 'package:test/test.dart';
void customTest(String name, dynamic Function() testFn) => test(name, testFn);
''',
});
});

test('the root suite from an absolute path', () {
final path = p.prettyUri(p.join(d.sandbox, 'test.dart'));
return _expectReport(
'''
customTest('success 1', () {});
test('success 2', () {});
''',
useRelativePath: false,
[
[
suiteJson(0, path: equalsIgnoringCase(path)),
testStartJson(
1, allOf(startsWith('loading '), endsWith('test.dart')),
groupIDs: []),
testDoneJson(1, hidden: true),
],
[
groupJson(2, testCount: 2),
testStartJson(3, 'success 1',
line: 3,
column: 60,
url: p.toUri(p.join(d.sandbox, 'common.dart')).toString(),
rootColumn: 7,
rootLine: 7,
rootUrl: p.toUri(p.join(d.sandbox, 'test.dart')).toString()),
testDoneJson(3),
testStartJson(4, 'success 2', line: 8, column: 7),
testDoneJson(4),
]
],
doneJson(),
externalLibraries: {
'common.dart': '''
import 'package:test/test.dart';
void customTest(String name, dynamic Function() testFn) => test(name, testFn);
''',
});
Expand Down Expand Up @@ -586,6 +626,7 @@ void customTest(String name, dynamic Function() testFn) => test(name, testFn);
Future<void> _expectReport(String tests,
List<List<Object /*Map|Matcher*/ >> expected, Map<Object, Object> done,
{List<String> args = const [],
bool useRelativePath = true,
Map<String, String> externalLibraries = const {}}) async {
var testContent = StringBuffer('''
import 'dart:async';
Expand All @@ -603,8 +644,9 @@ import 'package:test/test.dart';
..writeln('}');

await d.file('test.dart', testContent.toString()).create();
var testPath = useRelativePath ? 'test.dart' : p.join(d.sandbox, 'test.dart');

var test = await runTest(['test.dart', '--chain-stack-traces', ...args],
var test = await runTest([testPath, '--chain-stack-traces', ...args],
reporter: 'json');
await test.shouldExit();

Expand Down
9 changes: 5 additions & 4 deletions pkgs/test/test/runner/json_reporter_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,16 @@ Map<String, Object> _allSuitesJson({int count = 1}) =>
/// Returns the event emitted by the JSON reporter indicating that a suite has
/// begun running.
///
/// The [platform] defaults to `"vm"`, the [path] defaults to `"test.dart"`.
/// The [platform] defaults to `'vm'`.
/// The [path] defaults to `equals('test.dart')`.
Map<String, Object> suiteJson(int id,
{String platform = 'vm', String path = 'test.dart'}) =>
{String platform = 'vm', Matcher? path}) =>
{
'type': 'suite',
'suite': {
'id': id,
'platform': platform,
'path': path,
'path': path ?? 'test.dart',
}
};

Expand Down Expand Up @@ -120,7 +121,7 @@ Map<String, Object> groupJson(int id,
/// [skip] is `true`, the test is expected to be marked as skipped without a
/// reason. If it's a [String], the test is expected to be marked as skipped
/// with that reason.
Map<String, Object> testStartJson(int id, String name,
Map<String, Object> testStartJson(int id, Object /*String|Matcher*/ name,
{int? suiteID,
Iterable<int>? groupIDs,
int? line,
Expand Down
2 changes: 2 additions & 0 deletions pkgs/test_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* Drop support for null unsafe Dart, bump SDK constraint to `3.0.0`.
* Add `final` modifier on some implementation classes: `Configuration`,
`CustomRuntime`,`RuntimeSettings`, `SuiteConfiguration`.
* Fix the `root_` fields in the JSON reporter when running a test on Windows
with an absolute path.

## 0.5.3

Expand Down
4 changes: 2 additions & 2 deletions pkgs/test_core/lib/src/runner/reporter/json.dart
Original file line number Diff line number Diff line change
Expand Up @@ -303,15 +303,15 @@ class JsonReporter implements Reporter {
/// all be `null`.
Map<String, dynamic> _frameInfo(SuiteConfiguration suiteConfig, Trace? trace,
Runtime runtime, String suitePath) {
var absoluteSuitePath = p.absolute(suitePath);
var absoluteSuitePath = p.canonicalize(p.absolute(suitePath));
var frame = trace?.frames.first;
if (frame == null || (suiteConfig.jsTrace && runtime.isJS)) {
return {'line': null, 'column': null, 'url': null};
}

var rootFrame = trace?.frames.firstWhereOrNull((frame) =>
frame.uri.scheme == 'file' &&
frame.uri.toFilePath() == absoluteSuitePath);
p.canonicalize(frame.uri.toFilePath()) == absoluteSuitePath);
return {
'line': frame.line,
'column': frame.column,
Expand Down

0 comments on commit 3d5afed

Please sign in to comment.