Remove the intermediate updateFromCommandLine function with mocks

- mock `fs.writeFileSync` so that `updateFromCommandLine` can call `settings.save()` and unit tests don't do crazy things to the pref set. Signed-off-by: Eric Promislow <[email protected]>
rancher-sandbox · Apr 13, 2022 · 92470e6 · 92470e6
1 parent bedfbdc
commit 92470e6
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 17 deletions.
diff --git a/background.ts b/background.ts
@@ -109,7 +109,7 @@ Electron.app.whenReady().then(async() => {
     cfg = settings.init();
     if (commandLineArgs.length) {
       try {
-        cfg = settings.updateFromCommandLineAndSave(cfg, commandLineArgs);
+        cfg = settings.updateFromCommandLine(cfg, commandLineArgs);
       } catch (err) {
         console.log(`Failed to update command from argument ${ commandLineArgs } `, err);
       }

diff --git a/src/config/__tests__/settings.spec.ts b/src/config/__tests__/settings.spec.ts
@@ -1,11 +1,15 @@
+import fs from 'fs';
+
 import * as settings from '../settings';
 import { PathManagementStrategy } from '@/integrations/pathManager';
 import { RecursivePartial } from '~/utils/typeUtils';
 
 describe('updateFromCommandLine', () => {
   let prefs: settings.Settings;
+  let configDir: string;
 
   beforeEach(() => {
+    jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { });
     prefs = {
       version:    4,
       kubernetes: {
@@ -81,24 +85,24 @@ describe('updateFromCommandLine', () => {
     });
   });
 
-  test('no args should leave prefs unchanged', () => {
+  test('no command-line args should leave prefs unchanged', () => {
     const newPrefs = settings.updateFromCommandLine(prefs, []);
 
     expect(newPrefs).toMatchObject(prefs);
   });
 
   test('one option with embedded equal sign should change only one value', () => {
-    const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-version=1.23.5']);
+    const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-version=1.23.6']);
 
-    expect(newPrefs.kubernetes.version).toBe('1.23.5');
+    expect(newPrefs.kubernetes.version).toBe('1.23.6');
     newPrefs.kubernetes.version = prefs.kubernetes.version;
     expect(newPrefs).toMatchObject(prefs);
   });
 
   test('one option over two args should change only one value', () => {
-    const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-version', '1.23.5']);
+    const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-version', '1.23.7']);
 
-    expect(newPrefs.kubernetes.version).toBe('1.23.5');
+    expect(newPrefs.kubernetes.version).toBe('1.23.7');
     newPrefs.kubernetes.version = prefs.kubernetes.version;
     expect(newPrefs).toMatchObject(prefs);
   });
@@ -111,15 +115,15 @@ describe('updateFromCommandLine', () => {
     expect(newPrefs).toMatchObject(prefs);
   });
 
-  test('boolean option to implicit true should change only that value', () => {
+  test('boolean option set to implicit true should change only that value', () => {
     const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-suppressSudo']);
 
     expect(newPrefs.kubernetes.suppressSudo).toBeTruthy();
     newPrefs.kubernetes.suppressSudo = false;
     expect(newPrefs).toMatchObject(prefs);
   });
 
-  test('boolean option to false should change only that value', () => {
+  test('boolean option set to false should change only that value', () => {
     const newPrefs = settings.updateFromCommandLine(prefs, ['--kubernetes-options-traefik=false']);
 
     expect(newPrefs.kubernetes.options.traefik).toBeFalsy();

diff --git a/src/config/settings.ts b/src/config/settings.ts
@@ -159,14 +159,6 @@ export function getUpdatableNode(cfg: Settings, fqFieldAccessor: string): [Recur
   return null;
 }
 
-export function updateFromCommandLineAndSave(cfg: Settings, args: string[]): Settings {
-  const newConfig = updateFromCommandLine(cfg, args);
-
-  save(newConfig);
-
-  return newConfig;
-}
-
 // Export for testing purposes only
 export function updateFromCommandLine(cfg: Settings, args: string[]): Settings {
   let i = 0;
@@ -199,7 +191,7 @@ export function updateFromCommandLine(cfg: Settings, args: string[]): Settings {
     case 'boolean':
       // --some-boolean-setting ==> --some-boolean-setting=true
       if (!value) {
-        finalValue = 'true'; // JSON.parse to the boolean a few lines later.
+        finalValue = 'true'; // JSON.parse to boolean `true` a few lines later.
       }
       break;
     default:
@@ -230,6 +222,9 @@ export function updateFromCommandLine(cfg: Settings, args: string[]): Settings {
     (lhs as Record<string, any>)[finalFieldName] = finalValue;
     i += 1;
   }
+  if (lim > 0) {
+    save(cfg);
+  }
 
   return cfg;
 }