diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index 8644c43222..2ae622d37f 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -69,14 +69,17 @@ class FlutterGoldenFileComparator implements GoldenFileComparator { // Calculate the appropriate basedir for the current test context. const FileSystem fs = LocalFileSystem(); final Directory testDirectory = fs.directory(defaultComparator.basedir); - print('test: $testDirectory'); final Directory flutterRoot = fs.directory(Platform.environment[_kFlutterRootKey]); - print('flutter: $flutterRoot'); - final Directory goldenRoot = flutterRoot.childDirectory(fs.path.join('bin', 'cache', 'pkg', 'goldens')); - print('golden: $goldenRoot'); - final String testDirectoryRelativePath = fs.path.relative(testDirectory.path, from: flutterRoot.path); - print('testDRP: $testDirectoryRelativePath'); - print('FGFC instantiated with:${goldenRoot.childDirectory(testDirectoryRelativePath).uri}'); + final Directory goldenRoot = flutterRoot.childDirectory(fs.path.join( + 'bin', + 'cache', + 'pkg', + 'goldens', + )); + final String testDirectoryRelativePath = fs.path.relative( + testDirectory.path, + from: flutterRoot.path, + ); return FlutterGoldenFileComparator(goldenRoot.childDirectory(testDirectoryRelativePath).uri); } @@ -88,11 +91,13 @@ class FlutterGoldenFileComparator implements GoldenFileComparator { } final bool authorized = await _skiaClient.auth(fs.directory(basedir)); if (!authorized) { - //TODO(katelovett): Clean up for final implementation + //TODO(katelovett): Clean up for final implementation on CI return true; //throw test_package.TestFailure('Could not authorize golctl.'); } - return await _skiaClient.imgtest(golden.path, goldenFile); + await _skiaClient.imgtestInit(); + + return await _skiaClient.imgtestAdd(golden.path, goldenFile); } @override diff --git a/packages/flutter_goldens/test/flutter_goldens_test.dart b/packages/flutter_goldens/test/flutter_goldens_test.dart index 99e2ddb539..681a42a448 100644 --- a/packages/flutter_goldens/test/flutter_goldens_test.dart +++ b/packages/flutter_goldens/test/flutter_goldens_test.dart @@ -52,6 +52,13 @@ void main() { // check for successful auth - return true // check for unsuccessful auth - throw NonZeroExitCode // check for unavailable auth (not on CI) - return false + // check for redundant work + }); + + group('init', () { + // check for successful init - return true + // check for unsuccessful init - throw NonZeroExitCode + // Check for redundant work }); group('imgtest', () { diff --git a/packages/flutter_goldens_client/lib/client.dart b/packages/flutter_goldens_client/lib/client.dart index 98c207d31a..b0141f9d99 100644 --- a/packages/flutter_goldens_client/lib/client.dart +++ b/packages/flutter_goldens_client/lib/client.dart @@ -51,7 +51,12 @@ class SkiaGoldClient { Directory _workDirectory; - //TODO(katelovett): Environment variables swapped out for CI implementation + //TODO(katelovett): Environment variables are temporary for local testing + /// The local [Directory] where the Flutter repository is hosted. + /// + /// Uses the [fs] file system. + Directory get flutterRoot => fs.directory(platform.environment[_kFlutterRootKey]); + /// The [path] to the local [Directory] where the goldctl tool is hosted. /// /// Uses the [platform] [environment] in this iteration. @@ -68,11 +73,6 @@ class SkiaGoldClient { /// Uses the [platform] [environment] in this iteration. String get _skiaGoldInstance => platform.environment[_kSkiaGoldInstance]; - /// The local [Directory] where the Flutter repository is hosted. - /// - /// Uses the [fs] file system. - Directory get flutterRoot => fs.directory(platform.environment[_kFlutterRootKey]); - /// Prepares the local work space for golden file testing and initializes the /// goldctl authorization for executing tests. /// @@ -81,18 +81,24 @@ class SkiaGoldClient { _workDirectory = workDirectory; //TODO(katelovett): Cleanup for final CI implementation - if(_serviceAccount == null) - return false; // We are not in the proper environment for running these tests. + if (_serviceAccount == null) + return false; // Not in the proper environment for golden file testing. - final File authFile = io.File(path.join(_workDirectory.path, 'temp', 'auth_opt.json')); - if(!authFile.existsSync()) { + final File authFile = _workDirectory.childFile(fs.path.join( + 'temp', + 'auth_opt.json' + )); + if (!authFile.existsSync()) { final List authArguments = [ 'auth', '--service-account', _serviceAccount, '--work-dir', _workDirectory.childDirectory('temp').path, ]; - final io.ProcessResult authResults = io.Process.runSync(_goldctl, authArguments); + final io.ProcessResult authResults = io.Process.runSync( + _goldctl, + authArguments + ); if (authResults.exitCode != 0) { final StringBuffer buf = StringBuffer(); buf @@ -101,13 +107,14 @@ class SkiaGoldClient { ..writeln('stderr: ${authResults.stderr}'); throw NonZeroExitCode(authResults.exitCode, buf.toString()); } - } else { - print('The file is already here, skipping auth.'); } - // Run init - final File keysFile = io.File(path.join(_workDirectory.path, 'keys.json')); - if(!keysFile.existsSync()) { + return true; + } + Future imgtestInit() async { + final File keysFile = _workDirectory.childFile('keys.json'); + + if(!keysFile.existsSync() || await _isNewCommit()) { final String commitHash = await _getCommitHash(); final String keys = '${_workDirectory.path}keys.json'; final String failures = '${_workDirectory.path}failures.json'; @@ -124,6 +131,7 @@ class SkiaGoldClient { '--failure-file', failures, '--passfail', ]; + if(imgtestInitArguments.contains(null)) { final StringBuffer buf = StringBuffer(); buf.writeln('Null argument for Skia Gold imgtest init:'); @@ -133,8 +141,9 @@ class SkiaGoldClient { final io.ProcessResult imgtestInitResult = io.Process.runSync( _goldctl, - imgtestInitArguments + imgtestInitArguments, ); + if (imgtestInitResult.exitCode != 0) { final StringBuffer buf = StringBuffer(); buf @@ -143,14 +152,10 @@ class SkiaGoldClient { ..writeln('stderr: ${imgtestInitResult.stderr}'); throw NonZeroExitCode(imgtestInitResult.exitCode, buf.toString()); } - } else{ - print('Already init, skipping.'); } - return true; } - Future imgtest(String testName, File goldenFile) async { - + Future imgtestAdd(String testName, File goldenFile) async { final List imgtestArguments = [ 'imgtest', 'add', '--work-dir', _workDirectory.childDirectory('temp').path, @@ -165,7 +170,10 @@ class SkiaGoldClient { throw NonZeroExitCode(1, buf.toString()); } - final io.ProcessResult imgtestResult = io.Process.runSync(_goldctl, imgtestArguments); + final io.ProcessResult imgtestResult = io.Process.runSync( + _goldctl, + imgtestArguments, + ); if (imgtestResult.exitCode != 0) { final StringBuffer buf = StringBuffer(); buf @@ -176,13 +184,12 @@ class SkiaGoldClient { ..writeln('stderr: ${imgtestResult.stderr}'); throw NonZeroExitCode(imgtestResult.exitCode, buf.toString()); } - // print('PASS'); return true; } Future _getCommitHash() async { - // TODO(katelovett): Remove after pre-commit tests can be ingested - return '0572f158fb10505b840281124d07f8785d4f13f0'; + // TODO(katelovett): Remove after pre-commit tests can be ingested by Skia Gold + return 'd4e4726ac262b9f78b002b696f915f552fed3fc8'; // if (!flutterRoot.existsSync()) { // return null; // } else { @@ -191,7 +198,20 @@ class SkiaGoldClient { // workingDirectory: flutterRoot.path, // ); // return revParse.exitCode == 0 ? revParse.stdout.trim() : null; - } + } + + Future _isNewCommit() async { + // auth file is there, need to check if we are on a new commit + final File resultFile = _workDirectory.childFile(fs.path.join( + 'temp', + 'result-state.json' + )); + final String contents = await resultFile.readAsString(); + final Map resultJSON = convert.json.decode(contents); + final String lastTestedCommit = resultJSON['SharedConfig']['gitHash']; + final String currentCommit = await _getCommitHash(); + return lastTestedCommit == currentCommit ? false : true; + } String _getKeysJSON() { // TODO(katelovett): Parse out cleaner key information