Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move paths tracking to rdctl #4160

Merged

Conversation

adamkpickering
Copy link
Member

Closes #3652.

@adamkpickering adamkpickering force-pushed the 3652-move-paths-to-rdctl branch 9 times, most recently from 89dab9e to 7da11cb Compare June 26, 2023 22:46
@adamkpickering adamkpickering marked this pull request as ready for review June 26, 2023 22:49
@ericpromislow
Copy link
Contributor

Unit tests fail for pkg/rancher-desktop/utils/__tests__/paths.spec.ts on macOS:

Expected: "/Library/Preferences"
Received: "/Library/Preferences/rancher-desktop"
<Click to see difference>
    at /Users/ericp/workspace/rancher/desktop/pkg/rancher-desktop/utils/__tests__/paths.spec.ts:128:22

Needed change:

diff --git a/pkg/rancher-desktop/utils/__tests__/paths.spec.ts b/pkg/rancher-desktop/utils/__tests__/paths.spec.ts
index 5bf0e170..7cf8464d 100644
--- a/pkg/rancher-desktop/utils/__tests__/paths.spec.ts
+++ b/pkg/rancher-desktop/utils/__tests__/paths.spec.ts
@@ -80,7 +80,7 @@ describe('paths', () => {
     deploymentProfileSystem: {
       win32:  new Error('Windows profiles will be read from Registry'),
       linux:  '/etc/rancher-desktop',
-      darwin: '/Library/Preferences',
+      darwin: '/Library/Preferences/rancher-desktop',
     },
     deploymentProfileUser: {
       win32:  new Error('Windows profiles will be read from Registry'),

@ericpromislow
Copy link
Contributor

Also breakage on Windows:


 FAIL  pkg/rancher-desktop/main/__tests__/deploymentProfiles.spec.ts (6.893 s)
  deployment profiles
    windows deployment profiles
      profile
        defaults
          happy paths
            √ converts a single string into an array (71 ms)
            no system profiles, no user profiles
              √ loads nothing (39 ms)
            no system profiles, both user profiles
              × loads both profiles (208 ms)
          error paths
            √ loads a bad profile, complains about all the errors, and keeps only valid entries (125 ms)
    non-windows deployment profiles
      ○ skipped complains about invalid default values
      ○ skipped complains about invalid locked settings

  ● deployment profiles › windows deployment profiles › profile › defaults › happy paths › no system profiles, both user profiles › loads both profiles

    expect(received).toEqual(expected) // deep equality

    - Expected  - 8
    + Received  + 0

    @@ -8,18 +8,10 @@
          },
        },
        "application": Object {
          "adminAccess": false,
          "debug": true,
    -       "installed": Object {
    -         "bellingham": "WA",
    -         "elko": "NV",
    -         "portland": "OR",
    -         "shasta": "CA",
    -       },
    -     },
          "telemetry": Object {
            "enabled": true,
          },
        },
        "containerEngine": Object {

      280 |               const profile = await readDeploymentProfiles(REGISTRY_PROFILE_PATHS);
      281 |
    > 282 |               expect(profile.defaults).toEqual(defaultUserProfile);

Fix:

diff --git a/pkg/rancher-desktop/main/__tests__/deploymentProfiles.spec.ts b/pkg/rancher-desktop/main/__tests__/deploymentProfiles.spec.ts
index 088ccf4f..e916002a 100644
--- a/pkg/rancher-desktop/main/__tests__/deploymentProfiles.spec.ts
+++ b/pkg/rancher-desktop/main/__tests__/deploymentProfiles.spec.ts
@@ -51,6 +51,12 @@ describe('deployment profiles', () => {
 [${ FULL_DEFAULTS_PATH }\\application\\Telemetry]
 "ENABLED"=dword:1

+[${ FULL_DEFAULTS_PATH }\\application\\extensions\\installed]
+"bellingham"="WA"
+"portland"="OR"
+"shasta"="CA"
+"elko"="NV"
+
 [${ FULL_DEFAULTS_PATH }\\CONTAINERENGINE]
 "name"="moby"

@@ -77,11 +83,6 @@ describe('deployment profiles', () => {
 "riviere du loup"=dword:0
 "magog"=dword:0

-[${ FULL_DEFAULTS_PATH }\\extensions]
-"bellingham"="WA"
-"portland"="OR"
-"shasta"="CA"
-"elko"="NV"
 `;

     const lockedUserRegFile = `Windows Registry Editor Version 5.00

This is a holdover from a recent change moving /extensions to /applications/extensions/installed but wasn't tested on windows

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core code looks fine, but tests need to be fixed for windows and macOS

Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
rdctl is now needed for typescript unit tests, so we
need to ensure it is present when the tests run.
Running npm run build does this.

Signed-off-by: Adam Pickering <adam.pickering@suse.com>
@ericpromislow
Copy link
Contributor

ericpromislow commented Jul 4, 2023

Still breakage on macOS. Should be:

diff --git a/src/go/rdctl/pkg/paths/paths_darwin.go b/src/go/rdctl/pkg/paths/paths_darwin.go
index b0afd3e8..3193777a 100644
--- a/src/go/rdctl/pkg/paths/paths_darwin.go
+++ b/src/go/rdctl/pkg/paths/paths_darwin.go
@@ -33,7 +33,7 @@ func GetPaths(getResourcesPathFuncs ...func() (string, error)) (Paths, error) {
                OldIntegration:          "/usr/local/bin",
                Integration:             filepath.Join(altAppHome, "bin"),
                DeploymentProfileSystem: filepath.Join("/Library", "Preferences", appName),
-               DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences", appName),
+               DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences"),
                ExtensionRoot:           filepath.Join(appHome, "extensions"),
        }
        paths.Logs = os.Getenv("RD_LOGS_DIR")

and

diff --git a/src/go/rdctl/pkg/paths/paths_darwin_test.go b/src/go/rdctl/pkg/paths/paths_darwin_test.go
index 83267b34..2ac7c169 100644
--- a/src/go/rdctl/pkg/paths/paths_darwin_test.go
+++ b/src/go/rdctl/pkg/paths/paths_darwin_test.go
@@ -24,7 +24,7 @@ func TestGetPaths(t *testing.T) {
                        OldIntegration:          "/usr/local/bin",
                        Resources:               fakeResourcesPath,
                        DeploymentProfileSystem: filepath.Join("/Library", "Preferences", appName),
-                       DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences", appName),
+                       DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences"),
                        ExtensionRoot:           filepath.Join(homeDir, "Library", "Application Support", appName, "extensions"),
                }
                actualPaths, err := GetPaths(mockGetResourcesPath)
@@ -54,7 +54,7 @@ func TestGetPaths(t *testing.T) {
                        OldIntegration:          "/usr/local/bin",
                        Resources:               fakeResourcesPath,
                        DeploymentProfileSystem: filepath.Join("/Library", "Preferences", appName),
-                       DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences", appName),
+                       DeploymentProfileUser:   filepath.Join(homeDir, "Library", "Preferences"),
                        ExtensionRoot:           filepath.Join(homeDir, "Library", "Application Support", appName, "extensions"),
                }
                actualPaths, err := GetPaths(mockGetResourcesPath)

@ericpromislow
Copy link
Contributor

ericpromislow commented Jul 4, 2023

I keep getting this wrong. I got it wrong in my last review.

I'm going with https://docs.rancherdesktop.io/getting-started/deployment#profile-format-and-location which is actually in sync with the spec ( #3751 ):

On macOS, the system profile is in /Library/Preferences and the user profile is in `/Library/Preferences/

On Linux, the system profile is in /etc/rancher-desktop/ and the user profile is in ~/.config.

In general, user profiles can't be in a rancher-desktop-specific directory, because they'll be cleared by factory-reset.

I think the difference for the system profiles is that almost all mac system profiles live in /Library/Preferences, while linux profiles often live in a subdir of /etc

Here's the old code, which got that right:

export class DarwinPaths ...
  readonly deploymentProfileSystem = path.join('/Library', 'Preferences');
  readonly deploymentProfileUser = path.join(os.homedir(), 'Library', 'Preferences');

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mac paths are hard to get correct

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mac paths still need fixing

Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
Signed-off-by: Adam Pickering <adam.pickering@suse.com>
@adamkpickering
Copy link
Member Author

Ok I made it so that everything lines up with the documentation. Hopefully the tests pass now.

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests work now, looks good

@ericpromislow ericpromislow merged commit ba5ba77 into rancher-sandbox:main Jul 5, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Address duplication of paths between rdctl and typescript paths module
2 participants