From 05fedd7c11d2b3bdf0efefd040aad7af56c855d6 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 11:05:26 -0800 Subject: [PATCH] Ignore EOF newline characters and added tests to `dir_contents_diff` tool (flutter/engine#50805) No issue filed, but was [discussed on discord](https://discord.com/channels/608014603317936148/608021010377080866/1209568840644567091). --- engine/src/flutter/testing/run_tests.py | 1 + .../lib/dir_contents_diff.dart | 16 +++- .../tools/dir_contents_diff/pubspec.yaml | 16 ++++ .../test/dir_contents_diff_test.dart | 77 +++++++++++++++++++ .../test/file_bad_missing.txt | 1 + .../test/file_bad_unexpected.txt | 3 + .../tools/dir_contents_diff/test/file_ok.txt | 2 + .../test/file_ok_eof_newline.txt | 2 + .../dir_contents_diff/test/fixtures/a.txt | 1 + .../dir_contents_diff/test/fixtures/b.txt | 1 + 10 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/file_bad_missing.txt create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/file_bad_unexpected.txt create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/file_ok.txt create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/file_ok_eof_newline.txt create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/fixtures/a.txt create mode 100644 engine/src/flutter/tools/dir_contents_diff/test/fixtures/b.txt diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py index 4658f007c00..5107ec49d18 100755 --- a/engine/src/flutter/testing/run_tests.py +++ b/engine/src/flutter/testing/run_tests.py @@ -922,6 +922,7 @@ def build_dart_host_test_list(build_dir): os.path.join(build_dir, 'dart-sdk', 'lib', 'libraries.json'), ], ), + (os.path.join('flutter', 'tools', 'dir_contents_diff'), []), (os.path.join('flutter', 'tools', 'engine_tool'), []), (os.path.join('flutter', 'tools', 'githooks'), []), (os.path.join('flutter', 'tools', 'header_guard_check'), []), diff --git a/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart b/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart index 49b485cd391..c9329beaa0b 100644 --- a/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart +++ b/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart @@ -70,8 +70,20 @@ int dirContentsDiff(String goldenPath, String dirPath) { final String dirListing = _generateDirListing(dirPath); tempFile.writeAsStringSync(dirListing); final ProcessResult diffResult = Process.runSync( - 'git', ['diff', '-p', goldenPath, tempFile.path], - runInShell: true, stdoutEncoding: utf8); + 'git', + [ + 'diff', + // If you manually edit the golden file, many text editors will add + // trailing whitespace. This flag ignores that because honestly it's + // not a significant part of this test. + '--ignore-space-at-eol', + '-p', + goldenPath, + tempFile.path, + ], + runInShell: true, + stdoutEncoding: utf8, + ); if (diffResult.exitCode != 0) { print( 'Unexpected diff in $goldenPath, use `git apply` with the following patch.\n'); diff --git a/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml b/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml index 852177e328a..f1f6dd20de4 100644 --- a/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml +++ b/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml @@ -1,3 +1,19 @@ name: dir_contents_diff environment: sdk: '>=3.2.0-0 <4.0.0' +dev_dependencies: + litetest: any + path: any +dependency_overrides: + async_helper: + path: ../../../third_party/dart/pkg/async_helper + expect: + path: ../../../third_party/dart/pkg/expect + litetest: + path: ../../testing/litetest + meta: + path: ../../../third_party/dart/pkg/meta + path: + path: ../../../third_party/dart/third_party/pkg/path + smith: + path: ../../../third_party/dart/pkg/smith diff --git a/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart b/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart index e69de29bb2d..4bfa559c511 100644 --- a/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart +++ b/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart @@ -0,0 +1,77 @@ +import 'dart:io' as io; + +import 'package:litetest/litetest.dart'; +import 'package:path/path.dart' as p; + +void main() { + // Find a path to `dir_contents_diff.dart` from the working directory. + final String pkgPath = io.File.fromUri(io.Platform.script).parent.parent.path; + final String binPath = p.join( + pkgPath, + 'bin', + 'dir_contents_diff.dart', + ); + + // As a sanity check, ensure that the file exists. + if (!io.File(binPath).existsSync()) { + io.stderr.writeln('Unable to find $binPath'); + io.exitCode = 1; + return; + } + + + // Runs `../bin/dir_contents_diff.dart` with the given arguments. + (int, String) runSync(String goldenPath, String dirPath) { + final io.ProcessResult result = io.Process.runSync( + 'dart', + [binPath, goldenPath, dirPath], + ); + return (result.exitCode, result.stdout ?? result.stderr); + } + + test('lists files and diffs successfully', () { + final String goldenPath = p.join(pkgPath, 'test', 'file_ok.txt'); + final String dirPath = p.join(pkgPath, 'test', 'fixtures'); + final (int exitCode, String output) = runSync(goldenPath, dirPath); + if (exitCode != 0) { + io.stderr.writeln('Expected exit code 0, got $exitCode'); + io.stderr.writeln(output); + } + expect(exitCode, 0); + }); + + test('lists files and diffs successfully, even with an EOF newline', () { + final String goldenPath = p.join(pkgPath, 'test', 'file_ok_eof_newline.txt'); + final String dirPath = p.join(pkgPath, 'test', 'fixtures'); + final (int exitCode, String output) = runSync(goldenPath, dirPath); + if (exitCode != 0) { + io.stderr.writeln('Expected exit code 0, got $exitCode'); + io.stderr.writeln(output); + } + expect(exitCode, 0); + }); + + test('diff fails when an expected file is missing', () { + final String goldenPath = p.join(pkgPath, 'test', 'file_bad_missing.txt'); + final String dirPath = p.join(pkgPath, 'test', 'fixtures'); + final (int exitCode, String output) = runSync(goldenPath, dirPath); + if (exitCode == 0) { + io.stderr.writeln('Expected non-zero exit code, got $exitCode'); + io.stderr.writeln(output); + } + expect(exitCode, 1); + expect(output, contains('+a.txt')); + }); + + test('diff fails when an unexpected file is present', () { + final String goldenPath = p.join(pkgPath, 'test', 'file_bad_unexpected.txt'); + final String dirPath = p.join(pkgPath, 'test', 'fixtures'); + final (int exitCode, String output) = runSync(goldenPath, dirPath); + if (exitCode == 0) { + io.stderr.writeln('Expected non-zero exit code, got $exitCode'); + io.stderr.writeln(output); + } + expect(exitCode, 1); + expect(output, contains('-c.txt')); + }); +} diff --git a/engine/src/flutter/tools/dir_contents_diff/test/file_bad_missing.txt b/engine/src/flutter/tools/dir_contents_diff/test/file_bad_missing.txt new file mode 100644 index 00000000000..1f482482efa --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/file_bad_missing.txt @@ -0,0 +1 @@ +b.txt diff --git a/engine/src/flutter/tools/dir_contents_diff/test/file_bad_unexpected.txt b/engine/src/flutter/tools/dir_contents_diff/test/file_bad_unexpected.txt new file mode 100644 index 00000000000..5be9c6d18f7 --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/file_bad_unexpected.txt @@ -0,0 +1,3 @@ +a.txt +b.txt +c.txt diff --git a/engine/src/flutter/tools/dir_contents_diff/test/file_ok.txt b/engine/src/flutter/tools/dir_contents_diff/test/file_ok.txt new file mode 100644 index 00000000000..87122ecd89a --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/file_ok.txt @@ -0,0 +1,2 @@ +a.txt +b.txt diff --git a/engine/src/flutter/tools/dir_contents_diff/test/file_ok_eof_newline.txt b/engine/src/flutter/tools/dir_contents_diff/test/file_ok_eof_newline.txt new file mode 100644 index 00000000000..87122ecd89a --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/file_ok_eof_newline.txt @@ -0,0 +1,2 @@ +a.txt +b.txt diff --git a/engine/src/flutter/tools/dir_contents_diff/test/fixtures/a.txt b/engine/src/flutter/tools/dir_contents_diff/test/fixtures/a.txt new file mode 100644 index 00000000000..0d4f7dd5141 --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/fixtures/a.txt @@ -0,0 +1 @@ +Will be referenced by a test. diff --git a/engine/src/flutter/tools/dir_contents_diff/test/fixtures/b.txt b/engine/src/flutter/tools/dir_contents_diff/test/fixtures/b.txt new file mode 100644 index 00000000000..0d4f7dd5141 --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/test/fixtures/b.txt @@ -0,0 +1 @@ +Will be referenced by a test.