From c3d7adb5770bed94ebdcb42ad386da8225196f4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 7 Nov 2023 14:38:09 +0200 Subject: [PATCH] Update file being read by the GUI The GUI must now read from the home directory. There was some complexity in disinguishing LocalAppData from AppData that has now become simpler, and related tests are no longer necessary. --- .../ubuntu_pro_for_windows_test.dart | 20 +++++++---- gui/packages/ubuntupro/lib/app.dart | 1 - .../ubuntupro/lib/core/agent_api_paths.dart | 16 +++------ .../lib/pages/startup/agent_monitor.dart | 3 +- .../test/core/agent_api_paths_part2_test.dart | 14 ++------ .../test/core/agent_api_paths_part3_test.dart | 22 ------------ .../test/core/agent_api_paths_part4_test.dart | 4 +-- .../test/core/agent_api_paths_test.dart | 19 +++------- .../test/startup/agent_monitor_test.dart | 36 +++++++++---------- .../test/startup/startup_page_test.dart | 1 - 10 files changed, 43 insertions(+), 93 deletions(-) delete mode 100644 gui/packages/ubuntupro/test/core/agent_api_paths_part3_test.dart diff --git a/gui/packages/ubuntupro/integration_test/ubuntu_pro_for_windows_test.dart b/gui/packages/ubuntupro/integration_test/ubuntu_pro_for_windows_test.dart index 1352094e7..7e0d457b2 100644 --- a/gui/packages/ubuntupro/integration_test/ubuntu_pro_for_windows_test.dart +++ b/gui/packages/ubuntupro/integration_test/ubuntu_pro_for_windows_test.dart @@ -27,22 +27,28 @@ void main() { return stack; }; - // A temporary directory mocking the $env:LocalAppData directory to sandbox our agent. - Directory? tmp; + // A temporary directory mocking the $env:UserProfile directory to sandbox our agent. + Directory? tmpHome; + Directory? tmpLocalAppData; setUpAll(() async { await YaruTestWindow.ensureInitialized(); - // Use a random place inside the build tree as the `LOCALAPPDATA` env variable for all test cases below. - tmp = await msixRootDir().createTemp('test-'); + // Use a random place inside the build tree as the `USERPROFILE` env variable for all test cases below. + tmpHome = await msixRootDir().createTemp('test-'); + + tmpLocalAppData = Directory(p.join(tmpHome!.path, 'AppData/Local')); + await tmpLocalAppData!.create(recursive: true); + Environment( overrides: { - 'LOCALAPPDATA': tmp!.path, + 'USERPROFILE': tmpHome!.path, + 'LOCALAPPDATA': tmpLocalAppData!.path, 'UP4W_ALLOW_STORE_PURCHASE': '1', }, ); }); - tearDownAll(() => tmp?.delete(recursive: true)); + tearDownAll(() => tmpHome?.delete(recursive: true)); group('no agent build', () { // Verifies that a proper message is displayed when the agent cannot be run. testWidgets( @@ -82,7 +88,7 @@ void main() { [p.basenameWithoutExtension(agentImageName)], ); } - File(p.join(tmp!.path, 'Ubuntu Pro', 'addr')).deleteSync(); + File(p.join(tmpHome!.path, 'Ubuntu Pro', 'addr')).deleteSync(); }); tearDownAll(() async { diff --git a/gui/packages/ubuntupro/lib/app.dart b/gui/packages/ubuntupro/lib/app.dart index 459aa09e2..2da4d873e 100644 --- a/gui/packages/ubuntupro/lib/app.dart +++ b/gui/packages/ubuntupro/lib/app.dart @@ -32,7 +32,6 @@ class Pro4WindowsApp extends StatelessWidget { onGenerateTitle: (context) => AppLocalizations.of(context).appTitle, home: Provider( create: (context) => AgentStartupMonitor( - appName: kAppName, addrFileName: kAddrFileName, agentLauncher: launch, clientFactory: defaultClient, diff --git a/gui/packages/ubuntupro/lib/core/agent_api_paths.dart b/gui/packages/ubuntupro/lib/core/agent_api_paths.dart index 162f7698b..d62e20879 100644 --- a/gui/packages/ubuntupro/lib/core/agent_api_paths.dart +++ b/gui/packages/ubuntupro/lib/core/agent_api_paths.dart @@ -5,21 +5,13 @@ import 'package:path/path.dart' as p; import 'environment.dart'; -/// Provides the full path of the "[appDir]/[filename]" file +/// Provides the full path of the "[filename]" file /// under the well known directory where the Windows Agent stores its local data. /// Returns null if that directory location cannot be determined from the environment. -String? agentAddrFilePath(String appDir, String filename) { - // The well-known package path_provider doesn't return the LOCALAPPDATA directory - // but the APPDATA, which is usually under %USERPROFILE%/AppData/Roaming instead of - // %USERPROFILE%/AppData/Local, which is where the agent is storing the support data. - final localAppDir = Environment.instance['LOCALAPPDATA']; +String? agentAddrFilePath(String filename) { + final localAppDir = Environment.instance['USERPROFILE']; if (localAppDir != null) { - return p.join(localAppDir, appDir, filename); - } - - final userProfile = Environment.instance['USERPROFILE']; - if (userProfile != null) { - return p.join(userProfile, 'AppData', 'Local', appDir, filename); + return p.join(localAppDir, filename); } return null; diff --git a/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart b/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart index 7d2b2ee68..9440758b5 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart @@ -49,12 +49,11 @@ typedef AgentApiCallback = FutureOr Function(AgentApiClient); class AgentStartupMonitor { AgentStartupMonitor({ - required String appName, required String addrFileName, required this.agentLauncher, required this.clientFactory, required this.onClient, - }) : _addrFilePath = agentAddrFilePath(appName, addrFileName); + }) : _addrFilePath = agentAddrFilePath(addrFileName); final String? _addrFilePath; diff --git a/gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart b/gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart index a35c7e111..5b19ae58d 100644 --- a/gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart +++ b/gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart @@ -3,7 +3,6 @@ import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; -import 'package:ubuntupro/core/agent_api_paths.dart'; import 'package:ubuntupro/core/environment.dart'; void main() { @@ -15,15 +14,8 @@ void main() { // that will cost only a branch on if-null. Maybe the compiler can optimize // that away. I'm not sure. final _ = Environment( - overrides: {'LOCALAPPDATA': Platform.environment['APPDATA']!}, + overrides: { + 'USERPROFILE': Platform.environment['USERPROFILE']!, + }, ); - - test('misleading environment', () { - const appName = 'AwesomeApp'; - - final dir = agentAddrFilePath(appName, 'addr')!; - - expect(dir.contains('Roaming'), isTrue); - expect(dir.contains(appName), isTrue); - }); } diff --git a/gui/packages/ubuntupro/test/core/agent_api_paths_part3_test.dart b/gui/packages/ubuntupro/test/core/agent_api_paths_part3_test.dart deleted file mode 100644 index 6da790084..000000000 --- a/gui/packages/ubuntupro/test/core/agent_api_paths_part3_test.dart +++ /dev/null @@ -1,22 +0,0 @@ -@TestOn('windows') - -import 'package:flutter_test/flutter_test.dart'; -import 'package:ubuntupro/core/agent_api_paths.dart'; -import 'package:ubuntupro/core/environment.dart'; - -void main() { - // Because this is a singleton (a static lasting as long as the application) - // we need to apply the overrides early in main and they will last until - // main exits. - final _ = Environment(overrides: {'LOCALAPPDATA': null}); - - test('fallback maintain invariants', () { - const appName = 'AwesomeApp'; - - final dir = agentAddrFilePath(appName, 'addr')!; - - expect(dir.contains('Roaming'), isFalse); - expect(dir.contains('Local'), isTrue); - expect(dir.contains(appName), isTrue); - }); -} diff --git a/gui/packages/ubuntupro/test/core/agent_api_paths_part4_test.dart b/gui/packages/ubuntupro/test/core/agent_api_paths_part4_test.dart index 1c1357509..463a0d51c 100644 --- a/gui/packages/ubuntupro/test/core/agent_api_paths_part4_test.dart +++ b/gui/packages/ubuntupro/test/core/agent_api_paths_part4_test.dart @@ -11,9 +11,7 @@ void main() { final _ = Environment(overrides: {'LOCALAPPDATA': null, 'USERPROFILE': null}); test('complete failure due environment', () { - const appName = 'AwesomeApp'; - - final dir = agentAddrFilePath(appName, 'addr'); + final dir = agentAddrFilePath('.ubuntupro'); expect(dir, isNull); }); diff --git a/gui/packages/ubuntupro/test/core/agent_api_paths_test.dart b/gui/packages/ubuntupro/test/core/agent_api_paths_test.dart index 8dde61415..f06a4ab85 100644 --- a/gui/packages/ubuntupro/test/core/agent_api_paths_test.dart +++ b/gui/packages/ubuntupro/test/core/agent_api_paths_test.dart @@ -5,16 +5,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:ubuntupro/core/agent_api_paths.dart'; void main() { - tearDownAll(() => File('./addr').deleteSync()); - test('dir should not contain "Roaming"', () { - const appName = 'AwesomeApp'; - - final dir = agentAddrFilePath(appName, 'addr')!; - - expect(dir.contains('Roaming'), isFalse); - expect(dir.contains('Local'), isTrue); - expect(dir.contains(appName), isTrue); - }); + tearDownAll(() => File('./.ubuntupro').deleteSync()); test('read port from line', () { const port = 56768; @@ -37,7 +28,7 @@ void main() { }); test('read port from addr file', () async { - const filePath = './addr'; + const filePath = './.ubuntupro'; const port = 56768; const line = '[::]:$port'; final addr = File(filePath); @@ -59,7 +50,7 @@ void main() { }); test('empty file', () async { - const filePath = './addr'; + const filePath = './.ubuntupro'; final addr = File(filePath); addr.writeAsStringSync(''); @@ -70,7 +61,7 @@ void main() { }); test('access denied', () async { - const filePath = './addr'; + const filePath = './.ubuntupro'; final addr = File(filePath); addr.writeAsStringSync(''); @@ -86,7 +77,7 @@ void main() { }); test('bad format', () async { - const filePath = './addr'; + const filePath = './.ubuntupro'; const port = 56768; const line = 'Hello World $port'; final addr = File(filePath); diff --git a/gui/packages/ubuntupro/test/startup/agent_monitor_test.dart b/gui/packages/ubuntupro/test/startup/agent_monitor_test.dart index 68e2d1132..23b52bd80 100644 --- a/gui/packages/ubuntupro/test/startup/agent_monitor_test.dart +++ b/gui/packages/ubuntupro/test/startup/agent_monitor_test.dart @@ -18,20 +18,24 @@ void main() { const kTimeout = Duration(seconds: 3); const kInterval = Duration(milliseconds: 100); - Directory? appDir; + Directory? homeDir; + setUpAll(() async { - // Overrides the LOCALAPPDATA value to point to a temporary directory and + // Overrides the USERPROFILE value to point to a temporary directory and // creates the agent directory inside it, where we should find the addr file. - // Returns the mocked LOCALAPPDATA value for later deletion. - final tmp = await Directory.current.createTemp(); + // Returns the mocked USERPROFILE value for later deletion. + final tmpHome = await Directory.current.createTemp(); + final _ = Environment( - overrides: {'LOCALAPPDATA': tmp.path}, + overrides: { + 'USERPROFILE': tmpHome.path, + }, ); - appDir = await Directory(p.join(tmp.path, kAppName)).create(); + homeDir = tmpHome; }); tearDownAll(() async { - await appDir?.parent.delete(recursive: true); + await homeDir?.delete(recursive: true); }); test('agent cannot start', () async { @@ -41,7 +45,6 @@ void main() { /// A launch request will always fail. agentLauncher: () async => false, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -59,7 +62,7 @@ void main() { }); test('ping non responsive', () async { - writeDummyAddrFile(appDir!); + writeDummyAddrFile(homeDir!); // Fakes a ping failure. final mockClient = MockAgentApiClient(); @@ -69,7 +72,6 @@ void main() { /// A launch request will always succeed. agentLauncher: () async => true, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -86,14 +88,13 @@ void main() { }); test('format error', () async { - writeDummyAddrFile(appDir!, line: 'Hello, 45567'); + writeDummyAddrFile(homeDir!, line: 'Hello, 45567'); final mockClient = MockAgentApiClient(); final monitor = AgentStartupMonitor( /// A launch request will always succeed. agentLauncher: () async => true, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -116,7 +117,6 @@ void main() { /// A launch request will always succeed. agentLauncher: () async => true, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -138,7 +138,7 @@ void main() { }); test('already running with mocks', () async { - writeDummyAddrFile(appDir!); + writeDummyAddrFile(homeDir!); final mockClient = MockAgentApiClient(); // Fakes a successful ping. @@ -147,7 +147,6 @@ void main() { /// A launch request will always succeed. agentLauncher: () async => true, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -170,11 +169,10 @@ void main() { final monitor = AgentStartupMonitor( /// A launch request will always succeed. agentLauncher: () async { - writeDummyAddrFile(appDir!); + writeDummyAddrFile(homeDir!); return true; }, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -201,7 +199,6 @@ void main() { return true; }, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) {}, ); @@ -225,11 +222,10 @@ void main() { final monitor = AgentStartupMonitor( /// A launch request will always succeed. agentLauncher: () async { - writeDummyAddrFile(appDir!); + writeDummyAddrFile(homeDir!); return true; }, clientFactory: (port) => mockClient, - appName: kAppName, addrFileName: kAddrFileName, onClient: (_) async { // This function only completes when the completer is manually set complete. diff --git a/gui/packages/ubuntupro/test/startup/startup_page_test.dart b/gui/packages/ubuntupro/test/startup/startup_page_test.dart index 20e27b874..99e703a65 100644 --- a/gui/packages/ubuntupro/test/startup/startup_page_test.dart +++ b/gui/packages/ubuntupro/test/startup/startup_page_test.dart @@ -92,7 +92,6 @@ void main() { final app = MaterialApp( home: Provider( create: (context) => AgentStartupMonitor( - appName: 'app name', addrFileName: 'anywhere', agentLauncher: () async => true, clientFactory: (port) =>