From 6e5237652d6767fe1f1e4a7022f950cea9f338b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 7 Nov 2023 12:15:50 +0200 Subject: [PATCH 01/14] Write address file at the home directory This directory is not virtualized. Moving the addr file here means we will not need a virtualization exeption. The rest of the LocalAppData contents need not be read by other applications, so they can stay there. We also no longer need to create the cache user directory, we can assume the home directory exists. Hence, so we can no longer test for it. I removed that test case and added a new one so that we have at least one error case. --- .../cmd/ubuntu-pro-agent/agent/agent.go | 4 +- .../cmd/ubuntu-pro-agent/agent/agent_test.go | 14 +++--- .../cmd/ubuntu-pro-agent/agent/export_test.go | 18 ++++---- windows-agent/internal/daemon/daemon.go | 28 +++++------- windows-agent/internal/daemon/daemon_test.go | 45 +++++++++---------- 5 files changed, 52 insertions(+), 57 deletions(-) diff --git a/windows-agent/cmd/ubuntu-pro-agent/agent/agent.go b/windows-agent/cmd/ubuntu-pro-agent/agent/agent.go index 505b8f0fa..3e0ad4ffc 100644 --- a/windows-agent/cmd/ubuntu-pro-agent/agent/agent.go +++ b/windows-agent/cmd/ubuntu-pro-agent/agent/agent.go @@ -42,7 +42,7 @@ type daemonConfig struct { } type options struct { - daemonCacheDir string + daemonAddrDir string proservicesCacheDir string registry config.Registry } @@ -111,7 +111,7 @@ func (a *App) serve(args ...option) error { } defer proservice.Stop(ctx) - daemon, err := daemon.New(ctx, proservice.RegisterGRPCServices, daemon.WithCacheDir(opt.daemonCacheDir)) + daemon, err := daemon.New(ctx, proservice.RegisterGRPCServices, daemon.WithAddrDir(opt.daemonAddrDir)) if err != nil { close(a.ready) return err diff --git a/windows-agent/cmd/ubuntu-pro-agent/agent/agent_test.go b/windows-agent/cmd/ubuntu-pro-agent/agent/agent_test.go index 1148bb394..4bc5d4029 100644 --- a/windows-agent/cmd/ubuntu-pro-agent/agent/agent_test.go +++ b/windows-agent/cmd/ubuntu-pro-agent/agent/agent_test.go @@ -124,10 +124,10 @@ func TestAppRunFailsOnComponentsCreationAndQuit(t *testing.T) { testCases := map[string]struct { invalidProServicesCache bool - invalidDaemonCache bool + invalidDaemonAddr bool }{ - "Invalid service cache": {invalidProServicesCache: true}, - "Invalid daemon cache": {invalidDaemonCache: true}, + "Invalid service cache": {invalidProServicesCache: true}, + "Invalid daemon address directory": {invalidDaemonAddr: true}, } for name, tc := range testCases { @@ -137,18 +137,18 @@ func TestAppRunFailsOnComponentsCreationAndQuit(t *testing.T) { badCache := filepath.Join(t.TempDir(), "file") - daemonCache := "" + daemonAddr := "" serviceCache := "" if tc.invalidProServicesCache { serviceCache = badCache } - if tc.invalidDaemonCache { - daemonCache = badCache + if tc.invalidDaemonAddr { + daemonAddr = badCache } - a := agent.NewForTesting(t, daemonCache, serviceCache) + a := agent.NewForTesting(t, daemonAddr, serviceCache) a.SetArgs() err := os.WriteFile(badCache, []byte("I'm here to break the service"), 0600) diff --git a/windows-agent/cmd/ubuntu-pro-agent/agent/export_test.go b/windows-agent/cmd/ubuntu-pro-agent/agent/export_test.go index 8c0288af1..af163be44 100644 --- a/windows-agent/cmd/ubuntu-pro-agent/agent/export_test.go +++ b/windows-agent/cmd/ubuntu-pro-agent/agent/export_test.go @@ -7,9 +7,9 @@ import ( "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" ) -func withDaemonCacheDir(dir string) func(*options) { +func withDaemonAddrDir(dir string) func(*options) { return func(o *options) { - o.daemonCacheDir = dir + o.daemonAddrDir = dir } } @@ -26,18 +26,18 @@ func withRegistry(r config.Registry) func(*options) { } // NewForTesting creates a new App with overridden paths for the service and daemon caches. -func NewForTesting(t *testing.T, daemonCacheDir, serviceCacheDir string) *App { +func NewForTesting(t *testing.T, daemonAddrDir, serviceCacheDir string) *App { t.Helper() - if daemonCacheDir == "" && serviceCacheDir == "" { + if daemonAddrDir == "" && serviceCacheDir == "" { // Common temp cache directory - daemonCacheDir = t.TempDir() - serviceCacheDir = daemonCacheDir - } else if daemonCacheDir == "" { - daemonCacheDir = t.TempDir() + daemonAddrDir = t.TempDir() + serviceCacheDir = daemonAddrDir + } else if daemonAddrDir == "" { + daemonAddrDir = t.TempDir() } else if serviceCacheDir == "" { serviceCacheDir = t.TempDir() } - return New(withProServicesCacheDir(serviceCacheDir), withDaemonCacheDir(daemonCacheDir), withRegistry(registry.NewMock())) + return New(withProServicesCacheDir(serviceCacheDir), withDaemonAddrDir(daemonAddrDir), withRegistry(registry.NewMock())) } diff --git a/windows-agent/internal/daemon/daemon.go b/windows-agent/internal/daemon/daemon.go index 163370e5d..9a6e14cdf 100644 --- a/windows-agent/internal/daemon/daemon.go +++ b/windows-agent/internal/daemon/daemon.go @@ -28,17 +28,17 @@ type Daemon struct { // options are the configurable functional options for the daemon. type options struct { - cacheDir string + addrDir string } // Option is the function signature we are passing to tweak the daemon creation. type Option func(*options) -// WithCacheDir overrides the cache directory used in the daemon. -func WithCacheDir(cachedir string) Option { +// WithAddrDir overrides the directory where the address file will be stored. +func WithAddrDir(addrDir string) Option { return func(o *options) { - if cachedir != "" { - o.cacheDir = cachedir + if addrDir != "" { + o.addrDir = addrDir } } } @@ -56,21 +56,17 @@ func New(ctx context.Context, registerGRPCServices GRPCServiceRegisterer, args . f(&opts) } - if opts.cacheDir == "" { - // Set default cache dir. - localAppData := os.Getenv("LocalAppData") - if localAppData == "" { - return d, errors.New("Could not read env variable LocalAppData") + if opts.addrDir == "" { + // Set default addr dir. + home := os.Getenv("UserProfile") + if home == "" { + return d, errors.New("Could not read env variable UserProfile") } - opts.cacheDir = filepath.Join(localAppData, common.LocalAppDataDir) + opts.addrDir = home } - // Create our cache directory if needed - if err := os.MkdirAll(opts.cacheDir, 0750); err != nil { - return d, err - } - listeningPortFilePath := filepath.Join(opts.cacheDir, common.ListeningPortFileName) + listeningPortFilePath := filepath.Join(opts.addrDir, common.ListeningPortFileName) log.Debugf(ctx, "Daemon port file path: %s", listeningPortFilePath) return Daemon{ diff --git a/windows-agent/internal/daemon/daemon_test.go b/windows-agent/internal/daemon/daemon_test.go index e136afcdd..664b802f4 100644 --- a/windows-agent/internal/daemon/daemon_test.go +++ b/windows-agent/internal/daemon/daemon_test.go @@ -21,26 +21,27 @@ import ( ) func TestNew(t *testing.T) { - t.Parallel() + // Not parallel because we use t.Setenv testCases := map[string]struct { - badCacheDir bool + badEnv bool wantErr bool }{ - "Success": {}, - "Error because of bad cache dir": {badCacheDir: true, wantErr: true}, + "Success": {}, + "Error because of empty %USERDIRECTORY%": {badEnv: true, wantErr: true}, } for name, tc := range testCases { tc := tc t.Run(name, func(t *testing.T) { - t.Parallel() + var args []daemon.Option - cacheDir := filepath.Join(t.TempDir(), "cache") - if tc.badCacheDir { - err := os.WriteFile(cacheDir, []byte("this is here to break the daemon"), 0600) - require.NoError(t, err, "Setup: failed to create broken directory as file") + if tc.badEnv { + t.Setenv("UserProfile", "") + } else { + addrDir := filepath.Join(t.TempDir(), "cache") + args = append(args, daemon.WithAddrDir(addrDir)) } var regCount int @@ -49,7 +50,7 @@ func TestNew(t *testing.T) { return nil } - _, err := daemon.New(context.Background(), countRegistrations, daemon.WithCacheDir(cacheDir)) + _, err := daemon.New(context.Background(), countRegistrations, args...) if tc.wantErr { require.Error(t, err, "New should have errored out but hasn't") return @@ -57,8 +58,6 @@ func TestNew(t *testing.T) { require.NoError(t, err, "New() should have return no error") require.Equal(t, 1, regCount, "daemon should register GRPC services only once") - - require.DirExists(t, cacheDir, "Cache dir should've been created in New") }) } } @@ -83,12 +82,12 @@ func TestStartQuit(t *testing.T) { t.Parallel() ctx := context.Background() - cacheDir := t.TempDir() + addrDir := t.TempDir() if tc.preexistingPortFile { - err := os.MkdirAll(cacheDir, 0600) + err := os.MkdirAll(addrDir, 0600) require.NoError(t, err, "Setup: failed to create pre-exisiting cache directory") - err = os.WriteFile(filepath.Join(cacheDir, common.ListeningPortFileName), []byte("# Old port file"), 0600) + err = os.WriteFile(filepath.Join(addrDir, common.ListeningPortFileName), []byte("# Old port file"), 0600) require.NoError(t, err, "Setup: failed to create pre-existing port file") } @@ -99,7 +98,7 @@ func TestStartQuit(t *testing.T) { return server } - d, err := daemon.New(ctx, registerer, daemon.WithCacheDir(cacheDir)) + d, err := daemon.New(ctx, registerer, daemon.WithAddrDir(addrDir)) require.NoError(t, err, "New() should return the daemon handler") serveErr := make(chan error) @@ -107,7 +106,7 @@ func TestStartQuit(t *testing.T) { serveErr <- d.Serve(ctx) }() - addrPath := filepath.Join(cacheDir, common.ListeningPortFileName) + addrPath := filepath.Join(addrDir, common.ListeningPortFileName) var addrContents []byte if tc.preexistingPortFile { @@ -175,18 +174,18 @@ func TestServeError(t *testing.T) { t.Parallel() ctx := context.Background() - cacheDir := t.TempDir() + addrDir := t.TempDir() registerer := func(context.Context) *grpc.Server { return grpc.NewServer() } - d, err := daemon.New(ctx, registerer, daemon.WithCacheDir(cacheDir)) + d, err := daemon.New(ctx, registerer, daemon.WithAddrDir(addrDir)) require.NoError(t, err, "New should return the daemon handler") defer d.Quit(ctx, false) // Remove parent directory to prevent listening port file to be written - require.NoError(t, os.RemoveAll(cacheDir), "Setup: could not remove cache directory") + require.NoError(t, os.RemoveAll(addrDir), "Setup: could not remove cache directory") err = d.Serve(ctx) require.Error(t, err, "Serve should fail when the cache dir does not exist") @@ -196,13 +195,13 @@ func TestQuitBeforeServe(t *testing.T) { t.Parallel() ctx := context.Background() - cacheDir := t.TempDir() + addrDir := t.TempDir() registerer := func(context.Context) *grpc.Server { return grpc.NewServer() } - d, err := daemon.New(ctx, registerer, daemon.WithCacheDir(cacheDir)) + d, err := daemon.New(ctx, registerer, daemon.WithAddrDir(addrDir)) require.NoError(t, err, "New should return the daemon handler") d.Quit(ctx, false) @@ -210,7 +209,7 @@ func TestQuitBeforeServe(t *testing.T) { err = d.Serve(ctx) require.Error(t, err, "Calling Serve() after Quit() should result in an error") - requireWaitPathDoesNotExist(t, filepath.Join(cacheDir, common.ListeningPortFileName), "Port file should not exist after returning from Serve()") + requireWaitPathDoesNotExist(t, filepath.Join(addrDir, common.ListeningPortFileName), "Port file should not exist after returning from Serve()") } // grpcPersistentCall will create a persistent GRPC connection to the server. From d5f73211926fa573b2d489a3340e84da37ae4dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 7 Nov 2023 12:16:08 +0200 Subject: [PATCH 02/14] Rename address file to .ubuntupro This file now lives in the home directory, so it's best to give it a less cryptic name. --- common/consts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/consts.go b/common/consts.go index 958009815..6f943863b 100644 --- a/common/consts.go +++ b/common/consts.go @@ -10,7 +10,7 @@ const ( LocalAppDataDir = "Ubuntu Pro" // ListeningPortFileName corresponds to the base name of the file hosting the addressing of our GRPC server. - ListeningPortFileName = "addr" + ListeningPortFileName = ".ubuntupro" // MsStoreProductID is the ID of the product in the Microsoft Store // From 0d188c096c0d214d5339357a65da14d984604afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 28 Nov 2023 08:58:53 +0100 Subject: [PATCH 03/14] Update WSL-Pro-Service internal dependency to "common" --- wsl-pro-service/go.mod | 2 +- wsl-pro-service/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/wsl-pro-service/go.mod b/wsl-pro-service/go.mod index 45d7e6530..a56d50f29 100644 --- a/wsl-pro-service/go.mod +++ b/wsl-pro-service/go.mod @@ -4,7 +4,7 @@ go 1.21.4 require ( github.com/canonical/ubuntu-pro-for-windows/agentapi v0.0.0-20231120144258-eaedf425f737 - github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075046-d228c858ed8a + github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075531-d5f73211926f github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231113084350-57c6acdb2d5b github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf github.com/sirupsen/logrus v1.9.3 diff --git a/wsl-pro-service/go.sum b/wsl-pro-service/go.sum index 05d021a58..311049a5b 100644 --- a/wsl-pro-service/go.sum +++ b/wsl-pro-service/go.sum @@ -40,8 +40,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/canonical/ubuntu-pro-for-windows/agentapi v0.0.0-20231120144258-eaedf425f737 h1:ng27r1HEpGqGmoaemcMiaOm2LyKiG9FnANVBdfL3DwU= github.com/canonical/ubuntu-pro-for-windows/agentapi v0.0.0-20231120144258-eaedf425f737/go.mod h1:btQ/eVeGFXdXdifpHRo4V05VmmnhyhWWkecmVnvUbfQ= -github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075046-d228c858ed8a h1:YBlQocZaMCZ5HugvmmRkwedN+Qn79dryZ1DUmpq5SJM= -github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075046-d228c858ed8a/go.mod h1:HWMzmvIFQ3dWWPE4paEDWOMpOkcjFb2akrccYrck+eM= +github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075531-d5f73211926f h1:pKSzODrpJ9e2aQGqqWEwSUnBRQpKFPKefkl5MdK3gus= +github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231128075531-d5f73211926f/go.mod h1:HWMzmvIFQ3dWWPE4paEDWOMpOkcjFb2akrccYrck+eM= github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231113084350-57c6acdb2d5b h1:hohD71N1nVvCQID1D+0ci6dZANSl9pDjnEV0Gpu8M+A= github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231113084350-57c6acdb2d5b/go.mod h1:b0s/D9Q81b42hO9zjFXs7xpMs4jltc+7GwIUGEv2/5U= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= From 615688e3decd3592634a02977d7656f5c9ed7ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 7 Nov 2023 12:17:20 +0200 Subject: [PATCH 04/14] Change location where the WSL Pro Service reads the address file --- .../cmd/wsl-pro-service/service/service.go | 6 +++--- wsl-pro-service/internal/system/system.go | 17 +++++++++-------- wsl-pro-service/internal/system/system_test.go | 14 +++++++------- .../internal/testutils/mock_executables.go | 14 +++++++------- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/wsl-pro-service/cmd/wsl-pro-service/service/service.go b/wsl-pro-service/cmd/wsl-pro-service/service/service.go index 53026b1db..35fa7f6cf 100644 --- a/wsl-pro-service/cmd/wsl-pro-service/service/service.go +++ b/wsl-pro-service/cmd/wsl-pro-service/service/service.go @@ -99,12 +99,12 @@ func (a *App) serve(args ...option) error { } if len(opt.agentPortFilePath) == 0 { - localAppData, err := opt.system.LocalAppData(ctx) + home, err := opt.system.UserProfileDir(ctx) if err != nil { close(a.ready) - return fmt.Errorf("Could not find $env:LocalAppData: %v", err) + return fmt.Errorf("Could not find $env:UserProfile: %v", err) } - opt.agentPortFilePath = filepath.Join(localAppData, common.LocalAppDataDir, common.ListeningPortFileName) + opt.agentPortFilePath = filepath.Join(home, common.ListeningPortFileName) } srv := wslinstanceservice.New(opt.system) diff --git a/wsl-pro-service/internal/system/system.go b/wsl-pro-service/internal/system/system.go index 55ae6ec91..3775157ca 100644 --- a/wsl-pro-service/internal/system/system.go +++ b/wsl-pro-service/internal/system/system.go @@ -147,16 +147,16 @@ func (s System) wslDistroName(ctx context.Context) (string, error) { return fields[3], nil } -// LocalAppData provides the path to Windows' local app data directory from WSL, -// usually `/mnt/c/Users/JohnDoe/AppData/Local`. -func (s *System) LocalAppData(ctx context.Context) (wslPath string, err error) { +// UserProfileDir provides the path to Windows' user profile directory from WSL, +// usually `/mnt/c/Users/JohnDoe/`. +func (s *System) UserProfileDir(ctx context.Context) (wslPath string, err error) { // Find folder where windows is mounted on cmdExe, err := s.findCmdExe() if err != nil { return wslPath, err } - exe, args := s.backend.CmdExe(cmdExe, "/C", "echo %LocalAppData%") + exe, args := s.backend.CmdExe(cmdExe, "/C", "echo %UserProfile%") //nolint:gosec //this function simply prepends the WSL path to "cmd.exe" to the args. out, err := exec.CommandContext(ctx, exe, args...).Output() if err != nil { @@ -165,16 +165,17 @@ func (s *System) LocalAppData(ctx context.Context) (wslPath string, err error) { // Path from Windows' perspective ( C:\Users\... ) // It must be converted to linux ( /mnt/c/Users/... ) - localAppDataWindows := strings.TrimSpace(string(out)) + winHome := strings.TrimSpace(string(out)) - exe, args = s.backend.WslpathExecutable("-ua", localAppDataWindows) + exe, args = s.backend.WslpathExecutable("-ua", winHome) //nolint:gosec //outside of tests, this function simply prepends "wslpath" to the args. out, err = exec.CommandContext(ctx, exe, args...).Output() if err != nil { return wslPath, fmt.Errorf("error: %v, stdout: %s", err, string(out)) } - localAppDataLinux := strings.TrimSpace(string(out)) - return s.Path(localAppDataLinux), nil + + winHomeLinux := strings.TrimSpace(string(out)) + return s.Path(winHomeLinux), nil } // Path converts an absolute path into one inside the mocked filesystem. diff --git a/wsl-pro-service/internal/system/system_test.go b/wsl-pro-service/internal/system/system_test.go index 592758adc..a041c8ae7 100644 --- a/wsl-pro-service/internal/system/system_test.go +++ b/wsl-pro-service/internal/system/system_test.go @@ -120,7 +120,7 @@ func TestInfo(t *testing.T) { } } -func TestLocalAppData(t *testing.T) { +func TestUserProfileDir(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -170,19 +170,19 @@ func TestLocalAppData(t *testing.T) { os.RemoveAll(cmdExePath) } - got, err := system.LocalAppData(context.Background()) + got, err := system.UserProfileDir(context.Background()) if tc.wantErr { - require.Error(t, err, "Expected LocalAppData to return an error") + require.Error(t, err, "Expected UserProfile to return an error") return } - require.NoError(t, err, "Expected LocalAppData to return no errors") + require.NoError(t, err, "Expected UserProfile to return no errors") // Validating CMD path require.Equal(t, cmdExePath, *system.CmdExeCache(), "Unexpected path for cmd.exe") - // Validating LocalAppData - wantSuffix := `/mnt/d/Users/TestUser/AppData/Local` - require.True(t, strings.HasSuffix(got, wantSuffix), "Unexpected value returned by LocalAppData.\nWant suffix: %s\nGot: %s", wantSuffix, got) + // Validating UserProfile + wantSuffix := `/mnt/d/Users/TestUser` + require.True(t, strings.HasSuffix(got, wantSuffix), "Unexpected value returned by UserProfileDir.\nWant suffix: %s\nGot: %s", wantSuffix, got) }) } } diff --git a/wsl-pro-service/internal/testutils/mock_executables.go b/wsl-pro-service/internal/testutils/mock_executables.go index 755a863f2..0ce483cc8 100644 --- a/wsl-pro-service/internal/testutils/mock_executables.go +++ b/wsl-pro-service/internal/testutils/mock_executables.go @@ -42,12 +42,12 @@ var ( // defaultWindowsMount is the default path used in tests to set the windows filesystem mount. defaultWindowsMount = "/mnt/d/" - // windowsLocalAppDataDir is the default path used in tests to store Windows agent data. - windowsLocalAppDataDir = `D:\Users\TestUser\AppData\Local\` - linuxLocalAppDataDir = "/mnt/d/Users/TestUser/AppData/Local/" + // windowsUserProfileDir is the default path used in tests to store the Windows agent address file. + windowsUserProfileDir = `D:\Users\TestUser\` + linuxUserProfileDir = "/mnt/d/Users/TestUser/" // defaultAddrFile is the default path used in tests to store the address of the Windows Agent service. - defaultAddrFile = filepath.Join(linuxLocalAppDataDir, common.LocalAppDataDir, common.ListeningPortFileName) + defaultAddrFile = filepath.Join(linuxUserProfileDir, common.ListeningPortFileName) //go:embed filesystem_defaults/os-release defaultOsReleaseContents []byte @@ -446,7 +446,7 @@ func WslPathMock(t *testing.T) { } stdout, ok := map[string]string{ - windowsLocalAppDataDir: linuxLocalAppDataDir, + windowsUserProfileDir: linuxUserProfileDir, `D:\Users\TestUser\certificate`: filepath.Join(defaultWindowsMount, "Users/TestUser/certificate"), }[argv[1]] @@ -492,7 +492,7 @@ func CmdExeMock(t *testing.T) { return exitBadUsage } - if argv[1] != "echo %LocalAppData%" { + if argv[1] != "echo %UserProfile%" { fmt.Fprintf(os.Stderr, "Mock not implemented for args %q\n", argv) return exitBadUsage } @@ -501,7 +501,7 @@ func CmdExeMock(t *testing.T) { return exitError } - fmt.Fprintln(os.Stdout, windowsLocalAppDataDir) + fmt.Fprintln(os.Stdout, windowsUserProfileDir) return exitOk }) } From d6b2f8394a6cc5557d1326f8a2c06b3e2c119906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 8 Nov 2023 16:50:34 +0200 Subject: [PATCH 05/14] WSL-Pro-Service: Rename fixture directory The previous commit renamed a test, this one renames that test's fixtures. --- .../proc/mounts | 0 .../error_when_cmd.exe_does_not_exist/proc/mounts | 0 .../success_with_a_single_9p_filesystem_mount/proc/mounts | 0 .../success_with_multiple_9p_filesystem_mounts/proc/mounts | 0 .../success_with_multiple_types_of_filesystem_mounts/proc/mounts | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename wsl-pro-service/internal/system/testdata/{TestLocalAppData => TestUserProfileDir}/error_finding_cmd.exe_because_there_is_no_windows_fs_in__proc_mounts/proc/mounts (100%) rename wsl-pro-service/internal/system/testdata/{TestLocalAppData => TestUserProfileDir}/error_when_cmd.exe_does_not_exist/proc/mounts (100%) rename wsl-pro-service/internal/system/testdata/{TestLocalAppData => TestUserProfileDir}/success_with_a_single_9p_filesystem_mount/proc/mounts (100%) rename wsl-pro-service/internal/system/testdata/{TestLocalAppData => TestUserProfileDir}/success_with_multiple_9p_filesystem_mounts/proc/mounts (100%) rename wsl-pro-service/internal/system/testdata/{TestLocalAppData => TestUserProfileDir}/success_with_multiple_types_of_filesystem_mounts/proc/mounts (100%) diff --git a/wsl-pro-service/internal/system/testdata/TestLocalAppData/error_finding_cmd.exe_because_there_is_no_windows_fs_in__proc_mounts/proc/mounts b/wsl-pro-service/internal/system/testdata/TestUserProfileDir/error_finding_cmd.exe_because_there_is_no_windows_fs_in__proc_mounts/proc/mounts similarity index 100% rename from wsl-pro-service/internal/system/testdata/TestLocalAppData/error_finding_cmd.exe_because_there_is_no_windows_fs_in__proc_mounts/proc/mounts rename to wsl-pro-service/internal/system/testdata/TestUserProfileDir/error_finding_cmd.exe_because_there_is_no_windows_fs_in__proc_mounts/proc/mounts diff --git a/wsl-pro-service/internal/system/testdata/TestLocalAppData/error_when_cmd.exe_does_not_exist/proc/mounts b/wsl-pro-service/internal/system/testdata/TestUserProfileDir/error_when_cmd.exe_does_not_exist/proc/mounts similarity index 100% rename from wsl-pro-service/internal/system/testdata/TestLocalAppData/error_when_cmd.exe_does_not_exist/proc/mounts rename to wsl-pro-service/internal/system/testdata/TestUserProfileDir/error_when_cmd.exe_does_not_exist/proc/mounts diff --git a/wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_a_single_9p_filesystem_mount/proc/mounts b/wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_a_single_9p_filesystem_mount/proc/mounts similarity index 100% rename from wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_a_single_9p_filesystem_mount/proc/mounts rename to wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_a_single_9p_filesystem_mount/proc/mounts diff --git a/wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_multiple_9p_filesystem_mounts/proc/mounts b/wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_multiple_9p_filesystem_mounts/proc/mounts similarity index 100% rename from wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_multiple_9p_filesystem_mounts/proc/mounts rename to wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_multiple_9p_filesystem_mounts/proc/mounts diff --git a/wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_multiple_types_of_filesystem_mounts/proc/mounts b/wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_multiple_types_of_filesystem_mounts/proc/mounts similarity index 100% rename from wsl-pro-service/internal/system/testdata/TestLocalAppData/success_with_multiple_types_of_filesystem_mounts/proc/mounts rename to wsl-pro-service/internal/system/testdata/TestUserProfileDir/success_with_multiple_types_of_filesystem_mounts/proc/mounts From dc4c153584fdf8440c96a043f2f6ac1c26759557 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 06/14] 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 | 3 +- gui/packages/ubuntupro/lib/constants.dart | 5 +-- .../ubuntupro/lib/core/agent_api_paths.dart | 18 +++------ .../lib/pages/startup/agent_monitor.dart | 11 +++-- .../lib/pages/startup/startup_model.dart | 4 +- .../test/core/agent_api_paths_part2_test.dart | 29 -------------- .../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 | 40 +++++++++---------- .../test/startup/startup_page_test.dart | 1 - 12 files changed, 51 insertions(+), 125 deletions(-) delete mode 100644 gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart 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 6c24c5ef4..2f2ea585d 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 @@ -28,22 +28,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( @@ -83,7 +89,7 @@ void main() { [p.basenameWithoutExtension(agentImageName)], ); } - File(p.join(tmp!.path, 'Ubuntu Pro', 'addr')).deleteSync(); + File(p.join(tmpHome!.path, '.ubuntupro')).deleteSync(); }); tearDownAll(() async { diff --git a/gui/packages/ubuntupro/lib/app.dart b/gui/packages/ubuntupro/lib/app.dart index 8cbbf9020..66f459cf0 100644 --- a/gui/packages/ubuntupro/lib/app.dart +++ b/gui/packages/ubuntupro/lib/app.dart @@ -23,7 +23,7 @@ class Pro4WindowsApp extends StatelessWidget { builder: (context, yaru, child) => ChangeNotifierProvider( create: (_) => ValueNotifier(SubscriptionInfo()), child: MaterialApp( - title: kAppName, + title: 'Ubuntu Pro', theme: yaru.theme, darkTheme: yaru.darkTheme, debugShowCheckedModeBanner: false, @@ -59,7 +59,6 @@ class Pro4WindowsApp extends StatelessWidget { Widget buildStartup(BuildContext context) { return Provider( create: (context) => AgentStartupMonitor( - appName: kAppName, addrFileName: kAddrFileName, agentLauncher: launch, clientFactory: defaultClient, diff --git a/gui/packages/ubuntupro/lib/constants.dart b/gui/packages/ubuntupro/lib/constants.dart index 13e1224eb..6f979b50f 100644 --- a/gui/packages/ubuntupro/lib/constants.dart +++ b/gui/packages/ubuntupro/lib/constants.dart @@ -1,11 +1,8 @@ /// The address where the Agent gRPC service is hosted. const kDefaultHost = '127.0.0.1'; -/// This app's name. Needed to find the Agent's addr file. -const kAppName = 'Ubuntu Pro'; - /// The name of the file where the Agent's drop its service connection information. -const kAddrFileName = 'addr'; +const kAddrFileName = '.ubuntupro'; /// The default border margin. const kDefaultMargin = 32.0; diff --git a/gui/packages/ubuntupro/lib/core/agent_api_paths.dart b/gui/packages/ubuntupro/lib/core/agent_api_paths.dart index 162f7698b..27e5b3dd7 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']; - 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); +String? agentAddrFilePath(String filename) { + final homeDir = Environment.instance['USERPROFILE']; + if (homeDir != null) { + return p.join(homeDir, 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..e36f8d421 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; @@ -68,9 +67,9 @@ class AgentStartupMonitor { final AgentApiCallback onClient; /// Models the background agent as seen by the GUI as a state machine, i.e.: - /// 1. Agent running state is checked (by looking for the `addr` file). + /// 1. Agent running state is checked (by looking for the `.ubuntpro` file). /// 2. Agent start is requested by calling [agentLaucher] if not running. - /// 3. Contents of the `addr` file are scanned periodically (between [interval]). + /// 3. Contents of the `.ubuntpro` file are scanned periodically (between [interval]). /// 4. When a port is available, [clientFactory] is called to create a new /// [AgentApiClient]. /// 5. When a PING request succeeds, the [onClient] function is called with @@ -156,9 +155,9 @@ class AgentStartupMonitor { return AgentState.pingNonResponsive; } - /// Assumes the agent crashed, i.e. the `addr` file exists but the agent + /// Assumes the agent crashed, i.e. the `.ubuntupro` file exists but the agent /// cannot respond to PING requets. - /// Thus, we delete the existing `addr` file and retry launching the agent. + /// Thus, we delete the existing `.ubuntupro` file and retry launching the agent. Future reset() async { if (_addrFilePath != null) { try { diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart index 8a68a6395..729ecbabf 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart @@ -62,8 +62,8 @@ class StartupModel extends ChangeNotifier { return completer.future; } - /// Assumes the agent crashed, i.e. the `addr` file exists but the agent cannot respond to PING requets. - /// Thus, we delete the existing `addr` file and try launching the agent. + /// Assumes the agent crashed, i.e. the `.ubuntupro` file exists but the agent cannot respond to PING requets. + /// Thus, we delete the existing `.ubuntupro` file and try launching the agent. Future resetAgent() async { assert( _view == ViewState.retry, 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 deleted file mode 100644 index a35c7e111..000000000 --- a/gui/packages/ubuntupro/test/core/agent_api_paths_part2_test.dart +++ /dev/null @@ -1,29 +0,0 @@ -@TestOn('windows') - -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() { - // Tests had to be split in different files because the Environment 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. - // The singleton is not an issue here because it's only meant for testing. - // In production it will always be initialized with the empty overrides and - // 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']!}, - ); - - 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..c686552d3 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. @@ -261,10 +257,10 @@ void main() { }); } -/// Writes a sample `addr` file to the destination containing either a proper +/// Writes a sample `.ubuntupro` file to the destination containing either a proper /// contents as if the agent would have written it or [line], if supplied. void writeDummyAddrFile(Directory appDir, {String? line}) { - final filePath = p.join(appDir.path, 'addr'); + final filePath = p.join(appDir.path, '.ubuntupro'); const port = 56789; const goodLine = '[::]:$port'; final addr = File(filePath); diff --git a/gui/packages/ubuntupro/test/startup/startup_page_test.dart b/gui/packages/ubuntupro/test/startup/startup_page_test.dart index ddbcd769a..56fcbb64d 100644 --- a/gui/packages/ubuntupro/test/startup/startup_page_test.dart +++ b/gui/packages/ubuntupro/test/startup/startup_page_test.dart @@ -98,7 +98,6 @@ void main() { final app = MaterialApp( home: Provider( create: (context) => AgentStartupMonitor( - appName: 'app name', addrFileName: 'anywhere', agentLauncher: () async => true, clientFactory: (port) => From 76e0b97b49ed96727d5ce758036d79faf9e79600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 8 Nov 2023 13:32:17 +0200 Subject: [PATCH 07/14] Disable virtualization exception for %LocalAppData% --- msix/UbuntuProForWindows/Package.appxmanifest | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/msix/UbuntuProForWindows/Package.appxmanifest b/msix/UbuntuProForWindows/Package.appxmanifest index 1a574d61a..fc4d01b1c 100644 --- a/msix/UbuntuProForWindows/Package.appxmanifest +++ b/msix/UbuntuProForWindows/Package.appxmanifest @@ -4,12 +4,10 @@ xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10" xmlns:desktop="http://schemas.microsoft.com/appx/manifest/desktop/windows10" xmlns:desktop2="http://schemas.microsoft.com/appx/manifest/desktop/windows10/2" - xmlns:desktop6="http://schemas.microsoft.com/appx/manifest/desktop/windows10/6" - xmlns:virtualization="http://schemas.microsoft.com/appx/manifest/virtualization/windows10" xmlns:uap="http://schemas.microsoft.com/appx/manifest/uap/windows10" xmlns:uap5="http://schemas.microsoft.com/appx/manifest/uap/windows10/5" xmlns:rescap="http://schemas.microsoft.com/appx/manifest/foundation/windows10/restrictedcapabilities" - IgnorableNamespaces="desktop desktop2 desktop6 virtualization uap uap5 rescap"> + IgnorableNamespaces="desktop desktop2 uap uap5 rescap"> Ubuntu Pro For Windows Canonical Group Limited Images\StoreLogo.png - - disabled - - - - $(KnownFolder:LocalAppData)\Ubuntu Pro - - @@ -84,6 +74,5 @@ - From f6c7688045a7af5af9dfba8edbbff282534653e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 8 Nov 2023 13:52:42 +0200 Subject: [PATCH 08/14] Update E2E to use the new file locations The E2E test cleans up the LocalAppData folder to avoid test coupling. This commit updates this cleanup to the changes to the new path for the address file. We also watch that file to know if the agent is serving already. This commit updates the path being watched. The log finder now searches in the virtualized directory. --- end-to-end/main_test.go | 78 ++++++++++++++++++--------- end-to-end/manual_token_input_test.go | 2 +- end-to-end/organization_token_test.go | 2 +- end-to-end/purchase_test.go | 5 +- end-to-end/utils_test.go | 44 ++++++++++----- 5 files changed, 89 insertions(+), 42 deletions(-) diff --git a/end-to-end/main_test.go b/end-to-end/main_test.go index 8b629033e..fedebb3d1 100644 --- a/end-to-end/main_test.go +++ b/end-to-end/main_test.go @@ -66,7 +66,7 @@ func TestMain(m *testing.M) { log.Fatalf("Setup: %v\n", err) } - if err := assertCleanLocalAppData(); err != nil { + if err := assertCleanFilesystem(); err != nil { log.Fatalf("Setup: %v\n", err) } @@ -228,45 +228,75 @@ func powershellf(ctx context.Context, command string, args ...any) *exec.Cmd { "-Command", fmt.Sprintf(`$env:PsModulePath="" ; `+command, args...)) } -// assertCleanLocalAppData returns error if directory '%LocalAppData%/Ubuntu Pro' exists. +// assertCleanFilesystem returns error if directory '%LocalAppData%/Ubuntu Pro' exists. // If safety checks are overridden, then the directory is removed and no error is returned. -func assertCleanLocalAppData() error { - path := os.Getenv("LocalAppData") - if path == "" { - return errors.New("variable $env:LocalAppData should not be empty") +func assertCleanFilesystem() error { + if os.Getenv(overrideSafety) != "" { + return cleanupFilesystem() } - path = filepath.Join(path, "Ubuntu Pro") + fileList, err := filesToCleanUp() + if err != nil { + return err + } - _, err := os.Stat(path) - if errors.Is(err, fs.ErrNotExist) { - return nil + var errs error + for _, path := range fileList { + _, err := os.Stat(path) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + errs = errors.Join(errs, fmt.Errorf("could not stat %q: %v", path, err)) + continue + } + + errs = errors.Join(errs, fmt.Errorf("path %q should not exist. Remove it from your machine "+ + "to agree to run this potentially destructive test.", path)) } + + return nil +} + +func cleanupFilesystem() error { + fileList, err := filesToCleanUp() if err != nil { - return fmt.Errorf("could not stat %q: %v", path, err) + return err } - if os.Getenv(overrideSafety) != "" { - return cleanupLocalAppData() + var errs error + for _, path := range fileList { + if err := os.RemoveAll(path); err != nil { + errs = errors.Join(errs, fmt.Errorf("could not clean up %s: %v", path, err)) + } } - return fmt.Errorf("Directory %q should not exist. Remove it from your machine "+ - "to agree to run this potentially destructive test.", path) + return errs } -// cleanupLocalAppData removes directory '%LocalAppData%/Ubuntu Pro' and all its contents. -func cleanupLocalAppData() error { - path := os.Getenv("LocalAppData") - if path == "" { - return errors.New("variable $env:LocalAppData should not be empty") +func filesToCleanUp() ([]string, error) { + fileList := []struct { + prefixEnv string + path string + }{ + {prefixEnv: "LocalAppData", path: "Ubuntu Pro"}, + {prefixEnv: "UserProfile", path: ".ubuntupro"}, + {prefixEnv: "UserProfile", path: ".ubuntupro.logs"}, } - path = filepath.Join(path, "Ubuntu Pro") - if err := os.RemoveAll(path); err != nil { - return fmt.Errorf("could not clean up LocalAppData: %v", err) + var out []string + var errs error + + for _, s := range fileList { + prefix := os.Getenv(s.prefixEnv) + if prefix == "" { + errs = errors.Join(errs, fmt.Errorf("variable $env:%s should not be empty", s.prefixEnv)) + } + + out = append(out, filepath.Join(prefix, s.path)) } - return nil + return out, errs } // assertCleanRegistry returns error if registry key 'UbuntuPro' exists. diff --git a/end-to-end/manual_token_input_test.go b/end-to-end/manual_token_input_test.go index acd0a5c56..15c3337f2 100644 --- a/end-to-end/manual_token_input_test.go +++ b/end-to-end/manual_token_input_test.go @@ -39,7 +39,7 @@ func TestManualTokenInput(t *testing.T) { ctx := context.Background() testSetup(t) - defer logWindowsAgentOnError(t) + defer logWindowsAgentOnError(t, ctx) // Either runs the ubuntupro app before... if tc.whenToken == beforeDistroRegistration { diff --git a/end-to-end/organization_token_test.go b/end-to-end/organization_token_test.go index a98339309..cbf594ef1 100644 --- a/end-to-end/organization_token_test.go +++ b/end-to-end/organization_token_test.go @@ -39,7 +39,7 @@ func TestOrganizationProvidedToken(t *testing.T) { ctx := context.Background() testSetup(t) - defer logWindowsAgentOnError(t) + defer logWindowsAgentOnError(t, ctx) if tc.whenToken == beforeDistroRegistration { activateOrgSubscription(t) diff --git a/end-to-end/purchase_test.go b/end-to-end/purchase_test.go index 66b700a27..79520dcb6 100644 --- a/end-to-end/purchase_test.go +++ b/end-to-end/purchase_test.go @@ -51,8 +51,10 @@ func TestPurchase(t *testing.T) { for name, tc := range testCases { tc := tc t.Run(name, func(t *testing.T) { + ctx := context.Background() + testSetup(t) - defer logWindowsAgentOnError(t) + defer logWindowsAgentOnError(t, ctx) settings := contractsmockserver.DefaultSettings() @@ -67,7 +69,6 @@ func TestPurchase(t *testing.T) { //nolint:errcheck // Nothing we can do about it defer cs.Stop() - ctx := context.Background() contractsCtx, contractsCancel := context.WithCancel(ctx) defer contractsCancel() diff --git a/end-to-end/utils_test.go b/end-to-end/utils_test.go index 342c4dc86..f341b2354 100644 --- a/end-to-end/utils_test.go +++ b/end-to-end/utils_test.go @@ -34,14 +34,14 @@ func testSetup(t *testing.T) { err = assertCleanRegistry() require.NoError(t, err, "Setup: registry is polluted, potentially by a previous test") - err = assertCleanLocalAppData() + err = assertCleanFilesystem() require.NoError(t, err, "Setup: local app data is polluted, potentially by a previous test") t.Cleanup(func() { err := errors.Join( stopAgent(ctx), cleanupRegistry(), - cleanupLocalAppData(), + cleanupFilesystem(), ) // Cannot assert: the test is finished already if err != nil { @@ -113,19 +113,16 @@ func startAgent(t *testing.T, ctx context.Context, arg string, environ ...string } }() - require.Eventually(t, func() bool { - localAppData := os.Getenv("LocalAppData") - if localAppData == "" { - t.Logf("Agent setup: $env:LocalAppData should not be empty") - return false - } + home := os.Getenv("UserProfile") + require.NotEmptyf(t, home, "Agent setup: $env:UserProfile should not be empty") - _, err := os.Stat(filepath.Join(localAppData, "Ubuntu Pro", "addr")) + require.Eventually(t, func() bool { + _, err := os.Stat(filepath.Join(home, ".ubuntupro")) if errors.Is(err, fs.ErrNotExist) { return false } if err != nil { - t.Logf("Agent setup: could not read addr file: %v", err) + t.Logf("Agent setup: could not read address file: %v", err) return false } return true @@ -198,7 +195,8 @@ func logWslProServiceOnError(t *testing.T, ctx context.Context, d gowsl.Distro) t.Logf("WSL Pro Service logs:\n%s\n", out) } -func logWindowsAgentOnError(t *testing.T) { +//nolint:revive // testing.T must precede the context +func logWindowsAgentOnError(t *testing.T, ctx context.Context) { t.Helper() if !t.Failed() { @@ -207,13 +205,31 @@ func logWindowsAgentOnError(t *testing.T) { localAppData := os.Getenv("LocalAppData") if localAppData == "" { - t.Log("could not access Windows Agent's logs: $env:LocalAppData is not assigned") + t.Log("could not find Windows Agent's logs: $env:LocalAppData is not assigned") return } - out, err := os.ReadFile(filepath.Join(localAppData, common.LocalAppDataDir, "log")) + // The virtualized LocalAppData is located under a path similar to this one: + // + // %LocalAppData%/Packages/CanonicalGroupLimited.UbuntuProForWindows_hhj52ngek5ykr/LocalCache/Local + // ^~~~~~~~~~~~~ + // This part changes from version to version + // + packageDir := filepath.Join(localAppData, "Packages", "CanonicalGroupLimited.UbuntuProForWindows_*") + + out, err := powershellf(ctx, "(Get-Item %q).FullName", packageDir).CombinedOutput() + if err != nil { + t.Logf("could not find Windows Agent's logs: could not find virtualized LocalAppData: %v", err) + return + } + + // Remove trailing endline + packageDir = strings.TrimSpace(string(out)) + + logsPath := filepath.Join(packageDir, "LocalCache", "Local", common.LocalAppDataDir, ".ubuntupro.log") + out, err = os.ReadFile(logsPath) if err != nil { - t.Logf("could not read Windows Agent's logs: %v", err) + t.Logf("could not read Windows Agent's logs at %q: %v", logsPath, err) return } From 8c45df824a4c518d418534fc3fdab976b071ee26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Mon, 27 Nov 2023 10:07:31 +0100 Subject: [PATCH 09/14] Reinstall the MSIX before every test This way we ensure we carry no state from one test to the next one --- end-to-end/main_test.go | 100 ++++++++++++++++++++++++------------- end-to-end/utils_test.go | 17 +++++++ tools/build/build-appx.ps1 | 9 +++- 3 files changed, 90 insertions(+), 36 deletions(-) diff --git a/end-to-end/main_test.go b/end-to-end/main_test.go index fedebb3d1..2ddc47f38 100644 --- a/end-to-end/main_test.go +++ b/end-to-end/main_test.go @@ -22,6 +22,9 @@ import ( var ( // testImagePath is the path to the test image. testImagePath string + + // msixPath is the path to the Ubuntu Pro For Windows MSIX. + msixPath string ) const ( @@ -49,6 +52,9 @@ const ( // referenceDistro is the WSL distro that will be used to generate the test image. referenceDistroAppx = "CanonicalGroupLimited.Ubuntu" + + // up4wAppxPackage is the Ubuntu Pro For Windows package. + up4wAppxPackage = "CanonicalGroupLimited.UbuntuProForWindows" ) func TestMain(m *testing.M) { @@ -80,9 +86,8 @@ func TestMain(m *testing.M) { log.Fatalf("Setup: %v\n", err) } - if err := assertAppxInstalled(ctx, "CanonicalGroupLimited.UbuntuProForWindows"); err != nil { - log.Fatalf("Setup: %v\n", err) - } + log.Printf("MSIX package located at %s", msixPath) + log.Printf("Deb package located at %s", wslProServiceDebPath) path, cleanup, err := generateTestImage(ctx, referenceDistro, wslProServiceDebPath) if err != nil { @@ -97,7 +102,7 @@ func TestMain(m *testing.M) { log.Printf("Cleanup: registry: %v\n", err) } - cmd := powershellf(ctx, "Get-AppxPackage -Name CanonicalGroupLimited.UbuntuProForWindows | Remove-AppxPackage") + cmd := powershellf(ctx, "Get-AppxPackage -Name %q | Remove-AppxPackage", up4wAppxPackage) if out, err := cmd.CombinedOutput(); err != nil { log.Printf("Cleanup: could not remove Appx: %v: %s", err, out) } @@ -106,24 +111,19 @@ func TestMain(m *testing.M) { func usePrebuiltProject(ctx context.Context) (debPath string, err error) { buildPath := os.Getenv(prebuiltPath) - // Remove Appx before installing - cmd := powershellf(ctx, "Get-AppxPackage CanonicalGroupLimited.UbuntuProForWindows | Remove-AppxPackage") - if out, err := cmd.CombinedOutput(); err != nil { - // (Probably because it was not installed) - log.Printf("Could not remove old AppxPackage: %v. %s", err, out) - } - - // Install Appx anew - cmd = powershellf(ctx, "Add-AppxPackage %q", filepath.Join(buildPath, "windows-agent", "UbuntuProForWindows_*.msixbundle")) - if out, err := cmd.CombinedOutput(); err != nil { - return "", fmt.Errorf("could not install pre-built AppxPackage: %v. %s", err, out) + // Locate the Appx package and store the path in global variable so that we can + // reinstall it before every test + result, err := globSingleResult(filepath.Join(buildPath, "windows-agent", "UbuntuProForWindows_*.msixbundle")) + if err != nil { + return "", fmt.Errorf("could not locate MSIX: %v", err) } + msixPath = result // Locate WSL-Pro-Service (it'll be installed later into the distros) debPath = filepath.Join(buildPath, "wsl-pro-service") - _, err = locateWslProServiceDeb(ctx, debPath) + _, err = locateWslProServiceDeb(debPath) if err != nil { - return "", fmt.Errorf("could not localte pre-built WSL-Pro-Service: %v", err) + return "", fmt.Errorf("could not locate pre-built WSL-Pro-Service: %v", err) } return debPath, err @@ -133,13 +133,24 @@ func buildProject(ctx context.Context) (debPath string, err error) { ctx, cancel := context.WithCancel(ctx) defer cancel() - debPath, err = os.MkdirTemp(os.TempDir(), "WslProService") + buildPath, err := os.MkdirTemp(os.TempDir(), "UP4W-E2E-build") if err != nil { - return "", fmt.Errorf("could not create temporary directory for debian artifacts") + return "", fmt.Errorf("could not create temporary directory for build artifacts") + } + + debPath = filepath.Join(buildPath, "wsl-pro-service") + winPath := filepath.Join(buildPath, "windows-agent") + + if err := os.MkdirAll(debPath, 0600); err != nil { + return "", fmt.Errorf("could not create directory for debian artifacts") + } + + if err := os.MkdirAll(winPath, 0600); err != nil { + return "", fmt.Errorf("could not create directory for MSIX artifacts") } jobs := map[string]*exec.Cmd{ - "Build Windows Agent": powershellf(ctx, `..\tools\build\build-appx.ps1 -Mode end_to_end_tests`), + "Build Windows Agent": powershellf(ctx, `..\tools\build\build-appx.ps1 -Mode end_to_end_tests -OutputDir %q`, winPath), "Build Wsl Pro Service": powershellf(ctx, `..\tools\build\build-deb.ps1 -OutputDir %q`, debPath), } @@ -178,8 +189,15 @@ func buildProject(ctx context.Context) (debPath string, err error) { return "", fmt.Errorf("could not build project: %v", err) } + // Locate the Appx package and store the path in global variable so that we can + // reinstall it before every test + path, err := globSingleResult(filepath.Join(winPath, "UbuntuProForWindows_*.msixbundle")) + if err != nil { + return "", fmt.Errorf("could not locate Appx: %v", err) + } + log.Println("Project built") - return debPath, nil + return path, nil } // assertAppxInstalled returns an error if the provided Appx is not installed. @@ -197,17 +215,12 @@ func assertAppxInstalled(ctx context.Context, appx string) error { } // locateWslProServiceDeb locates the WSL pro service at the repository root and returns its absolute path. -func locateWslProServiceDeb(ctx context.Context, path string) (debPath string, err error) { +func locateWslProServiceDeb(path string) (debPath string, err error) { defer decorate.OnError(&err, "could not locate wsl-pro-service deb package") - out, err := powershellf(ctx, `(Get-ChildItem -Path "%s/wsl-pro-service_*.deb").FullName`, path).CombinedOutput() + path, err = globSingleResult(filepath.Join(path, "wsl-pro-service_*.deb")) if err != nil { - return "", fmt.Errorf("could not read expected location: %v. %s", err, out) - } - - debPath = strings.TrimSpace(string(out)) - if debPath == "" { - return "", errors.New("Wsl Pro Service is not built") + return "", err } debPath, err = filepath.Abs(debPath) @@ -302,6 +315,10 @@ func filesToCleanUp() ([]string, error) { // assertCleanRegistry returns error if registry key 'UbuntuPro' exists. // If safety checks are overridden, then the key is removed and no error is returned. func assertCleanRegistry() error { + if os.Getenv(overrideSafety) != "" { + return cleanupRegistry() + } + k, err := registry.OpenKey(registry.CURRENT_USER, registryPath, registry.READ) if errors.Is(err, registry.ErrNotExist) { // Key does not exist, as expected @@ -314,11 +331,6 @@ func assertCleanRegistry() error { k.Close() - // Key exists: this is probably running outside of a clean runner - if os.Getenv(overrideSafety) != "" { - return cleanupRegistry() - } - // Protect unsuspecting users return fmt.Errorf(`UbuntuPro registry key should not exist. Remove it from your machine `+ `to agree to run this potentially destructive test. It can be located at `+ @@ -384,7 +396,7 @@ func generateTestImage(ctx context.Context, sourceDistro, wslProServiceDebPath s // From now on, all cleanups must be deferred because the distro // must be unregistered before removing the directory it is in. - debPath, err := locateWslProServiceDeb(ctx, wslProServiceDebPath) + debPath, err := locateWslProServiceDeb(wslProServiceDebPath) if err != nil { return "", nil, err } @@ -434,3 +446,21 @@ func assertDistroUnregistered(d gowsl.Distro) error { return nil } + +// globSingleResult searches for the specified glob pattern and returns success +// if, and only if, there is only one file matching it. +func globSingleResult(pattern string) (string, error) { + candidates, err := filepath.Glob(pattern) + if err != nil { + return "", fmt.Errorf("could not search pattern %s: %v", pattern, err) + } + + if len(candidates) == 0 { + return "", fmt.Errorf("no file matches pattern %s", pattern) + } + if len(candidates) > 1 { + return "", fmt.Errorf("multiple file match pattern %s:\n - %s", pattern, strings.Join(candidates, "\n - ")) + } + + return candidates[0], nil +} diff --git a/end-to-end/utils_test.go b/end-to-end/utils_test.go index f341b2354..18f2ec8da 100644 --- a/end-to-end/utils_test.go +++ b/end-to-end/utils_test.go @@ -31,6 +31,9 @@ func testSetup(t *testing.T) { err = stopAgent(ctx) require.NoError(t, err, "Setup: could not stop the agent") + err = reinstallMSIX(ctx, msixPath) + require.NoError(t, err, "Setup: could not reinstall the agent") + err = assertCleanRegistry() require.NoError(t, err, "Setup: registry is polluted, potentially by a previous test") @@ -235,3 +238,17 @@ func logWindowsAgentOnError(t *testing.T, ctx context.Context) { t.Logf("Windows Agent's logs:\n%s\n", out) } + +func reinstallMSIX(ctx context.Context, path string) error { + cmd := powershellf(ctx, "Get-AppxPackage %q | Remove-AppxPackage", up4wAppxPackage) + if out, err := cmd.CombinedOutput(); err != nil { + // (Probably because it was not installed) + log.Printf("Could not remove old AppxPackage: %v. %s", err, out) + } + + if out, err := powershellf(ctx, "Add-AppxPackage %q", path).CombinedOutput(); err != nil { + return fmt.Errorf("could not install AppxPackage: %v. %s", err, out) + } + + return nil +} diff --git a/tools/build/build-appx.ps1 b/tools/build/build-appx.ps1 index 9d778fd29..20e0a0cd9 100644 --- a/tools/build/build-appx.ps1 +++ b/tools/build/build-appx.ps1 @@ -5,8 +5,10 @@ param ( [Parameter(Mandatory = $true, HelpMessage = "production, end_to_end_tests.")] + [string]$mode, - [string]$mode + [Parameter(Mandatory = $false, HelpMessage = "A directory were the MSIX and the certificate will be copied to")] + [string]$OutputDir ) function Start-VsDevShell { @@ -78,6 +80,11 @@ function Install-Appx { | Select-Object -last 1 ` ) + if ( "${OutputDir}" -ne "" ) { + Copy-Item -Path "${artifacts}/*.cer" -Destination "${OutputDir}" + Copy-Item -Path "${artifacts}/*.msixbundle" -Destination "${OutputDir}" + } + If ($mode -ne 'production') { Add-AppxPackage "${env:ProgramFiles(x86)}\Microsoft SDKs\Windows Kits\10\ExtensionSDKs\Microsoft.VCLibs.Desktop\14.0\Appx\Debug\x64\Microsoft.VCLibs.x64.Debug.14.00.Desktop.appx" } From deef0e847e08e3b57a13b92786ae779becbbe569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Mon, 27 Nov 2023 11:45:32 +0100 Subject: [PATCH 10/14] Fix gRPC version in logstreamer internals Not sure where this comes from, probably a silent merge conflict --- wsl-pro-service/internal/grpc/logstreamer/test/log_test.pb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wsl-pro-service/internal/grpc/logstreamer/test/log_test.pb.go b/wsl-pro-service/internal/grpc/logstreamer/test/log_test.pb.go index ef59a5481..a7536f5ef 100644 --- a/wsl-pro-service/internal/grpc/logstreamer/test/log_test.pb.go +++ b/wsl-pro-service/internal/grpc/logstreamer/test/log_test.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.31.0 -// protoc v3.21.12 +// protoc v4.23.4 // source: log_test.proto package test From 79dc0f721c82ceba1e69ec42d91489c870f7da1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 28 Nov 2023 09:04:24 +0100 Subject: [PATCH 11/14] Use Go's glob rather than powershell when locating the agent's logs --- end-to-end/manual_token_input_test.go | 2 +- end-to-end/organization_token_test.go | 2 +- end-to-end/purchase_test.go | 2 +- end-to-end/utils_test.go | 15 +++++---------- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/end-to-end/manual_token_input_test.go b/end-to-end/manual_token_input_test.go index 15c3337f2..acd0a5c56 100644 --- a/end-to-end/manual_token_input_test.go +++ b/end-to-end/manual_token_input_test.go @@ -39,7 +39,7 @@ func TestManualTokenInput(t *testing.T) { ctx := context.Background() testSetup(t) - defer logWindowsAgentOnError(t, ctx) + defer logWindowsAgentOnError(t) // Either runs the ubuntupro app before... if tc.whenToken == beforeDistroRegistration { diff --git a/end-to-end/organization_token_test.go b/end-to-end/organization_token_test.go index cbf594ef1..a98339309 100644 --- a/end-to-end/organization_token_test.go +++ b/end-to-end/organization_token_test.go @@ -39,7 +39,7 @@ func TestOrganizationProvidedToken(t *testing.T) { ctx := context.Background() testSetup(t) - defer logWindowsAgentOnError(t, ctx) + defer logWindowsAgentOnError(t) if tc.whenToken == beforeDistroRegistration { activateOrgSubscription(t) diff --git a/end-to-end/purchase_test.go b/end-to-end/purchase_test.go index 79520dcb6..12406ce2d 100644 --- a/end-to-end/purchase_test.go +++ b/end-to-end/purchase_test.go @@ -54,7 +54,7 @@ func TestPurchase(t *testing.T) { ctx := context.Background() testSetup(t) - defer logWindowsAgentOnError(t, ctx) + defer logWindowsAgentOnError(t) settings := contractsmockserver.DefaultSettings() diff --git a/end-to-end/utils_test.go b/end-to-end/utils_test.go index 18f2ec8da..bdac1edb7 100644 --- a/end-to-end/utils_test.go +++ b/end-to-end/utils_test.go @@ -198,8 +198,7 @@ func logWslProServiceOnError(t *testing.T, ctx context.Context, d gowsl.Distro) t.Logf("WSL Pro Service logs:\n%s\n", out) } -//nolint:revive // testing.T must precede the context -func logWindowsAgentOnError(t *testing.T, ctx context.Context) { +func logWindowsAgentOnError(t *testing.T) { t.Helper() if !t.Failed() { @@ -218,19 +217,15 @@ func logWindowsAgentOnError(t *testing.T, ctx context.Context) { // ^~~~~~~~~~~~~ // This part changes from version to version // - packageDir := filepath.Join(localAppData, "Packages", "CanonicalGroupLimited.UbuntuProForWindows_*") - - out, err := powershellf(ctx, "(Get-Item %q).FullName", packageDir).CombinedOutput() + pattern := filepath.Join(localAppData, "Packages", "CanonicalGroupLimited.UbuntuProForWindows_*") + packageDir, err := globSingleResult(pattern) if err != nil { - t.Logf("could not find Windows Agent's logs: could not find virtualized LocalAppData: %v", err) + t.Logf("could not find Windows Agent's logs: could not locate MSIX virtualized directory: %v", err) return } - // Remove trailing endline - packageDir = strings.TrimSpace(string(out)) - logsPath := filepath.Join(packageDir, "LocalCache", "Local", common.LocalAppDataDir, ".ubuntupro.log") - out, err = os.ReadFile(logsPath) + out, err := os.ReadFile(logsPath) if err != nil { t.Logf("could not read Windows Agent's logs at %q: %v", logsPath, err) return From 25aaac3ae33ab75fc3b2a692c5acc74c0454515f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 28 Nov 2023 09:04:54 +0100 Subject: [PATCH 12/14] Move globSingleResult to the test_utils It's no longer used exclusively in main, so I moved it to utils. --- end-to-end/main_test.go | 18 ------------------ end-to-end/utils_test.go | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/end-to-end/main_test.go b/end-to-end/main_test.go index 2ddc47f38..6fe3bf51d 100644 --- a/end-to-end/main_test.go +++ b/end-to-end/main_test.go @@ -446,21 +446,3 @@ func assertDistroUnregistered(d gowsl.Distro) error { return nil } - -// globSingleResult searches for the specified glob pattern and returns success -// if, and only if, there is only one file matching it. -func globSingleResult(pattern string) (string, error) { - candidates, err := filepath.Glob(pattern) - if err != nil { - return "", fmt.Errorf("could not search pattern %s: %v", pattern, err) - } - - if len(candidates) == 0 { - return "", fmt.Errorf("no file matches pattern %s", pattern) - } - if len(candidates) > 1 { - return "", fmt.Errorf("multiple file match pattern %s:\n - %s", pattern, strings.Join(candidates, "\n - ")) - } - - return candidates[0], nil -} diff --git a/end-to-end/utils_test.go b/end-to-end/utils_test.go index bdac1edb7..bb9a9b25f 100644 --- a/end-to-end/utils_test.go +++ b/end-to-end/utils_test.go @@ -247,3 +247,21 @@ func reinstallMSIX(ctx context.Context, path string) error { return nil } + +// globSingleResult searches for the specified glob pattern and returns success +// if, and only if, there is only one file matching it. +func globSingleResult(pattern string) (string, error) { + candidates, err := filepath.Glob(pattern) + if err != nil { + return "", fmt.Errorf("could not search pattern %s: %v", pattern, err) + } + + if len(candidates) == 0 { + return "", fmt.Errorf("no file matches pattern %s", pattern) + } + if len(candidates) > 1 { + return "", fmt.Errorf("multiple file match pattern %s:\n - %s", pattern, strings.Join(candidates, "\n - ")) + } + + return candidates[0], nil +} From 42856d75e4524ee9e846e46de1fd4d9a303634fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 28 Nov 2023 09:06:41 +0100 Subject: [PATCH 13/14] Fix wrong environment variable in agent's test name Co-authored-by: Carlos Nihelton --- windows-agent/internal/daemon/daemon_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/windows-agent/internal/daemon/daemon_test.go b/windows-agent/internal/daemon/daemon_test.go index 664b802f4..406272fa9 100644 --- a/windows-agent/internal/daemon/daemon_test.go +++ b/windows-agent/internal/daemon/daemon_test.go @@ -28,8 +28,8 @@ func TestNew(t *testing.T) { wantErr bool }{ - "Success": {}, - "Error because of empty %USERDIRECTORY%": {badEnv: true, wantErr: true}, + "Success": {}, + "Error because of empty %USERPROFILE%": {badEnv: true, wantErr: true}, } for name, tc := range testCases { From ed0baa95bf7ee1ef7bba3cd99bc5fc23a946b22b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 28 Nov 2023 10:06:38 +0100 Subject: [PATCH 14/14] Improve error reports on package build --- end-to-end/main_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/end-to-end/main_test.go b/end-to-end/main_test.go index 6fe3bf51d..76581d6a4 100644 --- a/end-to-end/main_test.go +++ b/end-to-end/main_test.go @@ -142,11 +142,11 @@ func buildProject(ctx context.Context) (debPath string, err error) { winPath := filepath.Join(buildPath, "windows-agent") if err := os.MkdirAll(debPath, 0600); err != nil { - return "", fmt.Errorf("could not create directory for debian artifacts") + return "", fmt.Errorf("could not create directory for WSL-Pro-Service Debian package artifacts") } if err := os.MkdirAll(winPath, 0600); err != nil { - return "", fmt.Errorf("could not create directory for MSIX artifacts") + return "", fmt.Errorf("could not create directory for Ubuntu Pro For Windows MSIX artifacts") } jobs := map[string]*exec.Cmd{