do not warn about out-of-date Flutter installation too often (#9271) * do not warn about out-of-date Flutter installation too often * style fix
diff --git a/packages/flutter_tools/lib/src/version.dart b/packages/flutter_tools/lib/src/version.dart index f61327e..2a57429 100644 --- a/packages/flutter_tools/lib/src/version.dart +++ b/packages/flutter_tools/lib/src/version.dart
@@ -162,9 +162,18 @@ @visibleForTesting static final Duration kVersionAgeConsideredUpToDate = kCheckAgeConsideredUpToDate * 4; - /// The prefix of the stamp file where we cache Flutter version check data. + /// The amount of time we wait between issuing a warning. + /// + /// This is to avoid annoying users who are unable to upgrade right away. @visibleForTesting - static const String kFlutterVersionCheckStampFile = 'flutter_version_check'; + static const Duration kMaxTimeSinceLastWarning = const Duration(days: 1); + + /// The amount of time we pause for to let the user read the message about + /// outdated Flutter installation. + /// + /// This can be customized in tests to speed them up. + @visibleForTesting + static Duration kPauseToLetUserReadTheMessage = const Duration(seconds: 2); /// Checks if the currently installed version of Flutter is up-to-date, and /// warns the user if it isn't. @@ -185,8 +194,17 @@ return latestFlutterCommitDate.isAfter(localFrameworkCommitDate); } - if (installationSeemsOutdated && await newerFrameworkVersionAvailable()) + final VersionCheckStamp stamp = await VersionCheckStamp.load(); + final DateTime lastTimeWarningWasPrinted = stamp.lastTimeWarningWasPrinted ?? _clock.agoBy(kMaxTimeSinceLastWarning * 2); + final bool beenAWhileSinceWarningWasPrinted = _clock.now().difference(lastTimeWarningWasPrinted) > kMaxTimeSinceLastWarning; + + if (beenAWhileSinceWarningWasPrinted && installationSeemsOutdated && await newerFrameworkVersionAvailable()) { printStatus(versionOutOfDateMessage(frameworkAge), emphasis: true); + stamp.store( + newTimeWarningWasPrinted: _clock.now(), + ); + await new Future<Null>.delayed(kPauseToLetUserReadTheMessage); + } } @visibleForTesting @@ -213,26 +231,23 @@ /// unable to reach the server to get the latest version. Future<DateTime> _getLatestAvailableFlutterVersion() async { Cache.checkLockAcquired(); - const JsonEncoder kPrettyJsonEncoder = const JsonEncoder.withIndent(' '); - final String versionCheckStamp = Cache.instance.getStampFor(kFlutterVersionCheckStampFile); + final VersionCheckStamp versionCheckStamp = await VersionCheckStamp.load(); - if (versionCheckStamp != null) { - final Map<String, String> data = JSON.decode(versionCheckStamp); - final DateTime lastTimeVersionWasChecked = DateTime.parse(data['lastTimeVersionWasChecked']); - final Duration timeSinceLastCheck = _clock.now().difference(lastTimeVersionWasChecked); + if (versionCheckStamp.lastTimeVersionWasChecked != null) { + final Duration timeSinceLastCheck = _clock.now().difference(versionCheckStamp.lastTimeVersionWasChecked); // Don't ping the server too often. Return cached value if it's fresh. if (timeSinceLastCheck < kCheckAgeConsideredUpToDate) - return DateTime.parse(data['lastKnownRemoteVersion']); + return versionCheckStamp.lastKnownRemoteVersion; } // Cache is empty or it's been a while since the last server ping. Ping the server. try { final DateTime remoteFrameworkCommitDate = DateTime.parse(await FlutterVersion.fetchRemoteFrameworkCommitDate()); - Cache.instance.setStampFor(kFlutterVersionCheckStampFile, kPrettyJsonEncoder.convert(<String, String>{ - 'lastTimeVersionWasChecked': '${_clock.now()}', - 'lastKnownRemoteVersion': '$remoteFrameworkCommitDate', - })); + versionCheckStamp.store( + newTimeVersionWasChecked: _clock.now(), + newKnownRemoteVersion: remoteFrameworkCommitDate, + ); return remoteFrameworkCommitDate; } on VersionCheckError catch (error) { // This happens when any of the git commands fails, which can happen when @@ -244,6 +259,102 @@ } } +/// Contains data and load/save logic pertaining to Flutter version checks. +@visibleForTesting +class VersionCheckStamp { + /// The prefix of the stamp file where we cache Flutter version check data. + @visibleForTesting + static const String kFlutterVersionCheckStampFile = 'flutter_version_check'; + + const VersionCheckStamp({ + this.lastTimeVersionWasChecked, + this.lastKnownRemoteVersion, + this.lastTimeWarningWasPrinted, + }); + + final DateTime lastTimeVersionWasChecked; + final DateTime lastKnownRemoteVersion; + final DateTime lastTimeWarningWasPrinted; + + static Future<VersionCheckStamp> load() async { + final String versionCheckStamp = Cache.instance.getStampFor(kFlutterVersionCheckStampFile); + + if (versionCheckStamp != null) { + // Attempt to parse stamp JSON. + try { + final dynamic json = JSON.decode(versionCheckStamp); + if (json is Map) { + printTrace('Warning: expected version stamp to be a Map but found: $json'); + return fromJson(json); + } + } catch (error, stackTrace) { + // Do not crash if JSON is malformed. + printTrace('${error.runtimeType}: $error\n$stackTrace'); + } + } + + // Stamp is missing or is malformed. + return new VersionCheckStamp(); + } + + static VersionCheckStamp fromJson(Map<String, String> json) { + DateTime readDateTime(String property) { + return json.containsKey(property) + ? DateTime.parse(json[property]) + : null; + } + + return new VersionCheckStamp( + lastTimeVersionWasChecked: readDateTime('lastTimeVersionWasChecked'), + lastKnownRemoteVersion: readDateTime('lastKnownRemoteVersion'), + lastTimeWarningWasPrinted: readDateTime('lastTimeWarningWasPrinted'), + ); + } + + Future<Null> store({ + DateTime newTimeVersionWasChecked, + DateTime newKnownRemoteVersion, + DateTime newTimeWarningWasPrinted, + }) async { + final Map<String, String> jsonData = toJson(); + + if (newTimeVersionWasChecked != null) + jsonData['lastTimeVersionWasChecked'] = '$newTimeVersionWasChecked'; + + if (newKnownRemoteVersion != null) + jsonData['lastKnownRemoteVersion'] = '$newKnownRemoteVersion'; + + if (newTimeWarningWasPrinted != null) + jsonData['lastTimeWarningWasPrinted'] = '$newTimeWarningWasPrinted'; + + const JsonEncoder kPrettyJsonEncoder = const JsonEncoder.withIndent(' '); + Cache.instance.setStampFor(kFlutterVersionCheckStampFile, kPrettyJsonEncoder.convert(jsonData)); + } + + Map<String, String> toJson({ + DateTime updateTimeVersionWasChecked, + DateTime updateKnownRemoteVersion, + DateTime updateTimeWarningWasPrinted, + }) { + updateTimeVersionWasChecked = updateTimeVersionWasChecked ?? lastTimeVersionWasChecked; + updateKnownRemoteVersion = updateKnownRemoteVersion ?? lastKnownRemoteVersion; + updateTimeWarningWasPrinted = updateTimeWarningWasPrinted ?? lastTimeWarningWasPrinted; + + final Map<String, String> jsonData = <String, String>{}; + + if (updateTimeVersionWasChecked != null) + jsonData['lastTimeVersionWasChecked'] = '$updateTimeVersionWasChecked'; + + if (updateKnownRemoteVersion != null) + jsonData['lastKnownRemoteVersion'] = '$updateKnownRemoteVersion'; + + if (updateTimeWarningWasPrinted != null) + jsonData['lastTimeWarningWasPrinted'] = '$updateTimeWarningWasPrinted'; + + return jsonData; + } +} + /// Thrown when we fail to check Flutter version. /// /// This can happen when we attempt to `git fetch` but there is no network, or
diff --git a/packages/flutter_tools/test/src/version_test.dart b/packages/flutter_tools/test/src/version_test.dart index 47da067..fe8e351 100644 --- a/packages/flutter_tools/test/src/version_test.dart +++ b/packages/flutter_tools/test/src/version_test.dart
@@ -5,7 +5,6 @@ import 'dart:convert'; import 'package:collection/collection.dart'; -import 'package:meta/meta.dart'; import 'package:mockito/mockito.dart'; import 'package:process/process.dart'; import 'package:quiver/time.dart'; @@ -25,12 +24,12 @@ final DateTime _outOfDateVersion = _testClock.agoBy(FlutterVersion.kVersionAgeConsideredUpToDate * 2); final DateTime _stampUpToDate = _testClock.agoBy(FlutterVersion.kCheckAgeConsideredUpToDate ~/ 2); final DateTime _stampOutOfDate = _testClock.agoBy(FlutterVersion.kCheckAgeConsideredUpToDate * 2); -const String _stampMissing = '____stamp_missing____'; void main() { - group('FlutterVersion', () { + group('$FlutterVersion', () { setUpAll(() { Cache.disableLocking(); + FlutterVersion.kPauseToLetUserReadTheMessage = Duration.ZERO; }); testFlutterVersion('prints nothing when Flutter installation looks fresh', () async { @@ -44,12 +43,13 @@ fakeData( localCommitDate: _outOfDateVersion, - versionCheckStamp: _testStamp( + stamp: new VersionCheckStamp( lastTimeVersionWasChecked: _stampOutOfDate, lastKnownRemoteVersion: _outOfDateVersion, ), remoteCommitDate: _outOfDateVersion, expectSetStamp: true, + expectServerPing: true, ); await version.checkFlutterVersionFreshness(); @@ -61,28 +61,57 @@ fakeData( localCommitDate: _outOfDateVersion, - versionCheckStamp: _testStamp( + stamp: new VersionCheckStamp( lastTimeVersionWasChecked: _stampUpToDate, lastKnownRemoteVersion: _upToDateVersion, ), + expectSetStamp: true, ); await version.checkFlutterVersionFreshness(); _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion))); }); - testFlutterVersion('pings server when version stamp is missing', () async { + testFlutterVersion('does not print warning if printed recently', () async { final FlutterVersion version = FlutterVersion.instance; fakeData( localCommitDate: _outOfDateVersion, - versionCheckStamp: _stampMissing, - remoteCommitDate: _upToDateVersion, + stamp: new VersionCheckStamp( + lastTimeVersionWasChecked: _stampUpToDate, + lastKnownRemoteVersion: _upToDateVersion, + ), expectSetStamp: true, ); await version.checkFlutterVersionFreshness(); _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion))); + expect((await VersionCheckStamp.load()).lastTimeWarningWasPrinted, _testClock.now()); + + await version.checkFlutterVersionFreshness(); + _expectVersionMessage(''); + }); + + testFlutterVersion('pings server when version stamp is missing then does not', () async { + final FlutterVersion version = FlutterVersion.instance; + + fakeData( + localCommitDate: _outOfDateVersion, + remoteCommitDate: _upToDateVersion, + expectSetStamp: true, + expectServerPing: true, + ); + + await version.checkFlutterVersionFreshness(); + _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion))); + + // Immediate subsequent check is not expected to ping the server. + fakeData( + localCommitDate: _outOfDateVersion, + stamp: await VersionCheckStamp.load(), + ); + await version.checkFlutterVersionFreshness(); + _expectVersionMessage(''); }); testFlutterVersion('pings server when version stamp is out-of-date', () async { @@ -90,12 +119,13 @@ fakeData( localCommitDate: _outOfDateVersion, - versionCheckStamp: _testStamp( + stamp: new VersionCheckStamp( lastTimeVersionWasChecked: _stampOutOfDate, lastKnownRemoteVersion: _testClock.ago(days: 2), ), remoteCommitDate: _upToDateVersion, expectSetStamp: true, + expectServerPing: true, ); await version.checkFlutterVersionFreshness(); @@ -107,26 +137,98 @@ fakeData( localCommitDate: _outOfDateVersion, - versionCheckStamp: _stampMissing, errorOnFetch: true, + expectServerPing: true, ); await version.checkFlutterVersionFreshness(); _expectVersionMessage(''); }); }); + + group('$VersionCheckStamp', () { + void _expectDefault(VersionCheckStamp stamp) { + expect(stamp.lastKnownRemoteVersion, isNull); + expect(stamp.lastTimeVersionWasChecked, isNull); + expect(stamp.lastTimeWarningWasPrinted, isNull); + } + + testFlutterVersion('loads blank when stamp file missing', () async { + fakeData(); + _expectDefault(await VersionCheckStamp.load()); + }); + + testFlutterVersion('loads blank when stamp file is malformed JSON', () async { + fakeData(stampJson: '<'); + _expectDefault(await VersionCheckStamp.load()); + }); + + testFlutterVersion('loads blank when stamp file is well-formed but invalid JSON', () async { + fakeData(stampJson: '[]'); + _expectDefault(await VersionCheckStamp.load()); + }); + + testFlutterVersion('loads valid JSON', () async { + fakeData(stampJson: ''' + { + "lastKnownRemoteVersion": "${_testClock.ago(days: 1)}", + "lastTimeVersionWasChecked": "${_testClock.ago(days: 2)}", + "lastTimeWarningWasPrinted": "${_testClock.now()}" + } + '''); + + final VersionCheckStamp stamp = await VersionCheckStamp.load(); + expect(stamp.lastKnownRemoteVersion, _testClock.ago(days: 1)); + expect(stamp.lastTimeVersionWasChecked, _testClock.ago(days: 2)); + expect(stamp.lastTimeWarningWasPrinted, _testClock.now()); + }); + + testFlutterVersion('stores version stamp', () async { + fakeData(expectSetStamp: true); + + _expectDefault(await VersionCheckStamp.load()); + + final VersionCheckStamp stamp = new VersionCheckStamp( + lastKnownRemoteVersion: _testClock.ago(days: 1), + lastTimeVersionWasChecked: _testClock.ago(days: 2), + lastTimeWarningWasPrinted: _testClock.now(), + ); + await stamp.store(); + + final VersionCheckStamp storedStamp = await VersionCheckStamp.load(); + expect(storedStamp.lastKnownRemoteVersion, _testClock.ago(days: 1)); + expect(storedStamp.lastTimeVersionWasChecked, _testClock.ago(days: 2)); + expect(storedStamp.lastTimeWarningWasPrinted, _testClock.now()); + }); + + testFlutterVersion('overwrites individual fields', () async { + fakeData(expectSetStamp: true); + + _expectDefault(await VersionCheckStamp.load()); + + final VersionCheckStamp stamp = new VersionCheckStamp( + lastKnownRemoteVersion: _testClock.ago(days: 10), + lastTimeVersionWasChecked: _testClock.ago(days: 9), + lastTimeWarningWasPrinted: _testClock.ago(days: 8), + ); + await stamp.store( + newKnownRemoteVersion: _testClock.ago(days: 1), + newTimeVersionWasChecked: _testClock.ago(days: 2), + newTimeWarningWasPrinted: _testClock.now(), + ); + + final VersionCheckStamp storedStamp = await VersionCheckStamp.load(); + expect(storedStamp.lastKnownRemoteVersion, _testClock.ago(days: 1)); + expect(storedStamp.lastTimeVersionWasChecked, _testClock.ago(days: 2)); + expect(storedStamp.lastTimeWarningWasPrinted, _testClock.now()); + }); + }); } void _expectVersionMessage(String message) { final BufferLogger logger = context[Logger]; expect(logger.statusText.trim(), message.trim()); -} - -String _testStamp({@required DateTime lastTimeVersionWasChecked, @required DateTime lastKnownRemoteVersion}) { - return _kPrettyJsonEncoder.convert(<String, String>{ - 'lastTimeVersionWasChecked': '$lastTimeVersionWasChecked', - 'lastKnownRemoteVersion': '$lastKnownRemoteVersion', - }); + logger.clear(); } void testFlutterVersion(String description, dynamic testMethod()) { @@ -135,21 +237,24 @@ testMethod, overrides: <Type, Generator>{ FlutterVersion: () => new FlutterVersion(_testClock), - ProcessManager: () => new MockProcessManager(), - Cache: () => new MockCache(), }, ); } void fakeData({ - @required DateTime localCommitDate, + DateTime localCommitDate, DateTime remoteCommitDate, - String versionCheckStamp, - bool expectSetStamp: false, + VersionCheckStamp stamp, + String stampJson, bool errorOnFetch: false, + bool expectSetStamp: false, + bool expectServerPing: false, }) { - final MockProcessManager pm = context[ProcessManager]; - final MockCache cache = context[Cache]; + final MockProcessManager pm = new MockProcessManager(); + context.setVariable(ProcessManager, pm); + + final MockCache cache = new MockCache(); + context.setVariable(Cache, cache); ProcessResult success(String standardOutput) { return new ProcessResult(1, 0, standardOutput, ''); @@ -160,27 +265,22 @@ } when(cache.getStampFor(any)).thenAnswer((Invocation invocation) { - expect(invocation.positionalArguments.single, FlutterVersion.kFlutterVersionCheckStampFile); + expect(invocation.positionalArguments.single, VersionCheckStamp.kFlutterVersionCheckStampFile); - if (versionCheckStamp == _stampMissing) { - return null; - } + if (stampJson != null) + return stampJson; - if (versionCheckStamp != null) { - return versionCheckStamp; - } + if (stamp != null) + return JSON.encode(stamp.toJson()); - throw new StateError('Unexpected call to Cache.getStampFor(${invocation.positionalArguments}, ${invocation.namedArguments})'); + return null; }); when(cache.setStampFor(any, any)).thenAnswer((Invocation invocation) { - expect(invocation.positionalArguments.first, FlutterVersion.kFlutterVersionCheckStampFile); + expect(invocation.positionalArguments.first, VersionCheckStamp.kFlutterVersionCheckStampFile); if (expectSetStamp) { - expect(invocation.positionalArguments[1], _testStamp( - lastKnownRemoteVersion: remoteCommitDate, - lastTimeVersionWasChecked: _testClock.now(), - )); + stamp = VersionCheckStamp.fromJson(JSON.decode(invocation.positionalArguments[1])); return null; } @@ -205,6 +305,8 @@ } else if (argsAre('git', 'remote', 'add', '__flutter_version_check__', 'https://github.com/flutter/flutter.git')) { return success(''); } else if (argsAre('git', 'fetch', '__flutter_version_check__', 'master')) { + if (!expectServerPing) + fail('Did not expect server ping'); return errorOnFetch ? failure(128) : success(''); } else if (remoteCommitDate != null && argsAre('git', 'log', '__flutter_version_check__/master', '-n', '1', '--pretty=format:%ad', '--date=iso')) { return success(remoteCommitDate.toString());