feat: enhance environment variable persistence handling (#5783)

* feat: enhance environment variable persistence handling

* feat: experiment playwright with multiple workers

---------

Co-authored-by: Bijin Bruno <bijin@usebruno.com>

packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js

@@ -1467,8 +1467,16 @@ export const mergeAndPersistEnvironment =
         }
       });
 
-      // Save only non-ephemeral vars, or ephemerals explicitly persisted this run
+        // Save all non-ephemeral vars and all variables that were previously persisted
       const persistedNames = new Set(Object.keys(persistentEnvVariables));
+
+        // Add all existing non-ephemeral variables to persistedNames so they are preserved
+        existingVars.forEach((v) => {
+          if (!v.ephemeral) {
+            persistedNames.add(v.name);
+          }
+        });
+
       const environmentToSave = cloneDeep(environment);
       environmentToSave.variables = buildPersistedEnvVariables(merged, { mode: 'merge', persistedNames });
 

packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js

@@ -316,7 +316,7 @@ export const collectionsSlice = createSlice({
       }
     },
     scriptEnvironmentUpdateEvent: (state, action) => {
-      const { collectionUid, envVariables, runtimeVariables } = action.payload;
+      const { collectionUid, envVariables, runtimeVariables, persistentEnvVariables } = action.payload;
       const collection = findCollectionByUid(state.collections, collectionUid);
 
       if (collection) {
@@ -326,9 +326,10 @@ export const collectionsSlice = createSlice({
         if (activeEnvironment) {
           forOwn(envVariables, (value, key) => {
             const variable = find(activeEnvironment.variables, (v) => v.name === key);
+            const isPersistent = persistentEnvVariables && persistentEnvVariables[key] !== undefined;
 
             if (variable) {
-              // For updates coming from scripts, treat them as ephemeral overlays.
+              // For updates coming from scripts, treat them as ephemeral overlays unless they are persistent.
               if (variable.value !== value) {
                 /*
                  Overlay (persist: false): keep new value in Redux for UI and mark ephemeral
@@ -337,7 +338,7 @@ export const collectionsSlice = createSlice({
                 */
                 const previousValue = variable.value;
                 variable.value = value;
-                variable.ephemeral = true;
+                variable.ephemeral = !isPersistent;
                 if (variable.persistedValue === undefined) {
                   variable.persistedValue = previousValue;
                 }
@@ -353,7 +354,7 @@ export const collectionsSlice = createSlice({
                   enabled: true,
                   type: 'text',
                   uid: uuid(),
-                  ephemeral: true,
+                  ephemeral: !isPersistent
                 });
               }
             }

packages/bruno-electron/src/ipc/network/index.js

@@ -398,6 +398,7 @@ const registerNetworkIpc = (mainWindow) => {
       mainWindow.webContents.send('main:script-environment-update', {
         envVariables: scriptResult.envVariables,
         runtimeVariables: scriptResult.runtimeVariables,
+        persistentEnvVariables: scriptResult.persistentEnvVariables,
         requestUid,
         collectionUid
       });
@@ -486,6 +487,7 @@ const registerNetworkIpc = (mainWindow) => {
         mainWindow.webContents.send('main:script-environment-update', {
           envVariables: result.envVariables,
           runtimeVariables: result.runtimeVariables,
+          persistentEnvVariables: result.persistentEnvVariables,
           requestUid,
           collectionUid
         });
@@ -532,6 +534,7 @@ const registerNetworkIpc = (mainWindow) => {
       mainWindow.webContents.send('main:script-environment-update', {
         envVariables: scriptResult.envVariables,
         runtimeVariables: scriptResult.runtimeVariables,
+        persistentEnvVariables: scriptResult.persistentEnvVariables,
         requestUid,
         collectionUid
       });

playwright.config.ts

@@ -9,8 +9,8 @@ if (process.env.CI) {
 export default defineConfig({
   fullyParallel: false,
   forbidOnly: !!process.env.CI,
-  retries: process.env.CI ? 1 : 0,
-  workers: process.env.CI ? undefined : 1,
+  retries: process.env.CI ? 2 : 0,
+  workers: process.env.CI ? undefined : 3,
   reporter,
 
   use: {

tests/environments/api-setEnvVar/api-setEnvVar-with-persist.spec.ts

@@ -3,11 +3,9 @@ import fs from 'fs';
 import path from 'path';
 
 test.describe.serial('bru.setEnvVar(name, value, { persist: true })', () => {
-  test.setTimeout(2 * 10 * 1000);
-
   test('set env var with persist using script', async ({ pageWithUserData: page, restartApp }) => {
     // Keep a copy of the original Stage.bru file
-    const originalStageBruPath = path.join(__dirname, 'collection/environments/Stage.bru');
+    const originalStageBruPath = path.join(__dirname, 'fixtures/collection/environments/Stage.bru');
     const originalStageBruContent = fs.readFileSync(originalStageBruPath, 'utf8');
 
     // Select the collection and request
@@ -15,24 +13,25 @@ test.describe.serial('bru.setEnvVar(name, value, { persist: true })', () => {
     await page.getByText('api-setEnvVar-with-persist', { exact: true }).click();
 
     // open environment dropdown
-    await page.locator('div.current-environment').click();
+    await page.getByTestId('environment-selector-trigger').click();
 
     // select stage environment
-    await expect(page.locator('.dropdown-item').filter({ hasText: 'Stage' })).toBeVisible();
-    await page.locator('.dropdown-item').filter({ hasText: 'Stage' }).click();
-    await expect(page.locator('.current-environment').filter({ hasText: /Stage/ })).toBeVisible();
+    await expect(page.locator('.environment-list .dropdown-item', { hasText: 'Stage' })).toBeVisible();
+    await page.locator('.environment-list .dropdown-item', { hasText: 'Stage' }).click();
+    await expect(page.locator('.current-environment', { hasText: 'Stage' })).toBeVisible();
 
     // Send the request
-    await page.locator('#send-request').getByRole('img').nth(2).click();
-    await page.waitForTimeout(1000);
+    await page.getByTestId('send-arrow-icon').click();
+    await page.getByTestId('response-status-code').getByText(/200/).waitFor({ state: 'visible' });
+    await page.waitForTimeout(100);
 
     // confirm that the environment variable is set
-    await page.locator('div.current-environment').click();
-
-    await page.getByText('Configure', { exact: true }).click();
+    await page.getByTestId('environment-selector-trigger').click();
+    // open environment configuration
+    await page.locator('#configure-env').click();
     await expect(page.getByRole('row', { name: 'token' }).getByRole('cell').nth(1)).toBeVisible();
     await expect(page.getByRole('row', { name: 'secret' }).getByRole('cell').nth(2)).toBeVisible();
-    await page.getByText('×').click();
+    await page.getByTestId('modal-close-button').click();
 
     // we restart the app to confirm that the environment variable is persisted
     const newApp = await restartApp();
@@ -43,13 +42,13 @@ test.describe.serial('bru.setEnvVar(name, value, { persist: true })', () => {
     await newPage.getByText('api-setEnvVar-with-persist', { exact: true }).click();
 
     // open environment dropdown
-    await newPage.locator('div.current-environment').click();
-    await newPage.getByText('Configure', { exact: true }).click();
+    await newPage.getByTestId('environment-selector-trigger').click();
+    await newPage.locator('#configure-env').click();
     await expect(newPage.getByRole('row', { name: 'token' }).getByRole('cell').nth(1)).toBeVisible();
     await expect(newPage.getByRole('row', { name: 'secret' }).getByRole('cell').nth(2)).toBeVisible();
 
     // close the environment modal
-    await newPage.getByText('×').click();
+    await newPage.getByTestId('modal-close-button').click();
 
     // Restore the original Stage.bru file
     fs.writeFileSync(originalStageBruPath, originalStageBruContent);

tests/environments/api-setEnvVar/api-setEnvVar-without-persist.spec.ts

@@ -1,31 +1,30 @@
 import { test, expect } from '../../../playwright';
 
 test.describe.serial('bru.setEnvVar(name, value)', () => {
-  test.setTimeout(2 * 10 * 1000);
-
   test('set env var using script', async ({ pageWithUserData: page, restartApp }) => {
     // Select the collection and request
     await page.locator('#sidebar-collection-name').click();
     await page.getByText('api-setEnvVar-without-persist', { exact: true }).click();
 
     // open environment dropdown
-    await page.locator('div.current-environment').click();
+    await page.getByTestId('environment-selector-trigger').click();
 
     // select stage environment
-    await expect(page.locator('.dropdown-item').filter({ hasText: 'Stage' })).toBeVisible();
-    await page.locator('.dropdown-item').filter({ hasText: 'Stage' }).click();
-    await expect(page.locator('.current-environment').filter({ hasText: /Stage/ })).toBeVisible();
+    await expect(page.locator('.environment-list .dropdown-item', { hasText: 'Stage' })).toBeVisible();
+    await page.locator('.environment-list .dropdown-item', { hasText: 'Stage' }).click();
+    await expect(page.locator('.current-environment', { hasText: 'Stage' })).toBeVisible();
 
     // Send the request
-    await page.locator('#send-request').getByRole('img').nth(2).click();
-    await page.waitForTimeout(1000);
+    await page.getByTestId('send-arrow-icon').click();
+    await page.getByTestId('response-status-code').getByText(/200/).waitFor({ state: 'visible' });
+    await page.waitForTimeout(100);
 
     // confirm that the environment variable is set
-    await page.locator('div.current-environment').click();
-    await page.getByText('Configure', { exact: true }).click();
+    await page.getByTestId('environment-selector-trigger').click();
+    await page.locator('#configure-env').click();
     await expect(page.getByRole('row', { name: 'token' }).getByRole('cell').nth(1)).toBeVisible();
     await expect(page.getByRole('row', { name: 'secret' }).getByRole('cell').nth(2)).toBeVisible();
-    await page.getByText('×').click();
+    await page.getByTestId('modal-close-button').click();
 
     // we restart the app to confirm that the environment variable is not persisted
     const newApp = await restartApp();
@@ -36,14 +35,14 @@ test.describe.serial('bru.setEnvVar(name, value)', () => {
     await newPage.getByText('api-setEnvVar-without-persist', { exact: true }).click();
 
     // open environment dropdown
-    await newPage.locator('div.current-environment').click();
-    await newPage.getByText('Configure', { exact: true }).click();
+    await newPage.getByTestId('environment-selector-trigger').click();
+    await newPage.locator('#configure-env').click();
 
     // ensure that the environment variable is not persisted
     await expect(newPage.locator('table.environment-variables tbody')).not.toContainText('token');
 
     // close the environment variable modal
-    await newPage.getByText('×').click();
+    await newPage.getByTestId('modal-close-button').click();
     await newPage.close();
   });
 });

tests/environments/api-setEnvVar/collection/environments/Stage.bru

@@ -1,4 +0,0 @@
-vars {
-  host: https://testbench-sanity.usebruno.com
-  token: secret
-}

tests/environments/api-setEnvVar/fixtures/collection/environments/Stage.bru

@@ -0,0 +1,6 @@
+vars {
+  host: https://testbench-sanity.usebruno.com
+  token: secret
+  multiple-persist-vars-key1: value1
+  multiple-persist-vars-key2: value2
+}

tests/environments/api-setEnvVar/fixtures/collection/multiple-persist-vars-folder/folder.bru

@@ -0,0 +1,4 @@
+meta {
+  name: multiple-persist-vars-folder
+  type: folder
+}

tests/environments/api-setEnvVar/fixtures/collection/multiple-persist-vars-folder/multiple-persist-vars-1.bru

@@ -0,0 +1,15 @@
+meta {
+  name: multiple-persist-vars-1
+  type: http
+  seq: 1
+}
+
+get {
+  url: {{host}}/ping
+  body: none
+  auth: none
+}
+
+script:pre-request {
+  bru.setEnvVar("multiple-persist-vars-key1", "value1", { persist: true });
+}

tests/environments/api-setEnvVar/fixtures/collection/multiple-persist-vars-folder/multiple-persist-vars-2.bru

@@ -0,0 +1,15 @@
+meta {
+  name: multiple-persist-vars-2
+  type: http
+  seq: 2
+}
+
+get {
+  url: {{host}}/ping
+  body: none
+  auth: none
+}
+
+script:pre-request {
+  bru.setEnvVar("multiple-persist-vars-key2", "value2", { persist: true });
+}

tests/environments/api-setEnvVar/init-user-data/collection-security.json

@@ -1,7 +1,7 @@
 {
   "collections": [
     {
-      "path": "{{projectRoot}}/tests/environments/api-setEnvVar/collection",
+      "path": "{{projectRoot}}/tests/environments/api-setEnvVar/fixtures/collection",
       "securityConfig": {
         "jsSandboxMode": "safe"
       }

tests/environments/api-setEnvVar/init-user-data/preferences.json

@@ -1,6 +1,6 @@
 {
-    "maximized": true,
+    "maximized": false,
     "lastOpenedCollections": [
-        "{{projectRoot}}/tests/environments/api-setEnvVar/collection"
+        "{{projectRoot}}/tests/environments/api-setEnvVar/fixtures/collection"
     ]
 }

tests/environments/api-setEnvVar/multiple-persist-vars.spec.ts

@@ -0,0 +1,90 @@
+import { test, expect } from '../../../playwright';
+import fs from 'fs';
+import path from 'path';
+
+test.describe.serial('bru.setEnvVar multiple persistent variables', () => {
+  test.afterEach(async ({ pageWithUserData: page }) => {
+    // Clean up test environment variables after each test
+    try {
+      // Check if the page is still valid before attempting cleanup
+      if (page && !page.isClosed()) {
+        await page.locator('#sidebar-collection-name').click();
+        await page.getByTestId('environment-selector-trigger').click();
+        await page.locator('#configure-env').click();
+
+        // Remove the test environment variables
+        const key1Row = page.getByRole('row', { name: 'multiple-persist-vars-key1' });
+        if (await key1Row.isVisible()) {
+          await key1Row.getByRole('button').click(); // Click the delete button
+        }
+
+        const key2Row = page.getByRole('row', { name: 'multiple-persist-vars-key2' });
+        if (await key2Row.isVisible()) {
+          await key2Row.getByRole('button').click(); // Click the delete button
+        }
+
+        await page.getByTestId('modal-close-button').click();
+      }
+    } catch (error) {
+      // Ignore cleanup errors to avoid masking test failures
+      console.log('Cleanup failed:', error);
+    }
+  });
+
+  test('should persist multiple environment variables from different requests', async ({ pageWithUserData: page }) => {
+    await test.step('Select collection', async () => {
+      await page.locator('#sidebar-collection-name').click();
+      // The collection name should be 'collection' based on the test setup
+      await expect(page.locator('#sidebar-collection-name').filter({ hasText: 'collection' })).toBeVisible();
+    });
+
+    await test.step('Select stage environment', async () => {
+      await page.getByTestId('environment-selector-trigger').click();
+      await expect(page.locator('.environment-list .dropdown-item', { hasText: 'Stage' })).toBeVisible();
+      await page.locator('.environment-list .dropdown-item', { hasText: 'Stage' }).click();
+      await expect(page.locator('.current-environment', { hasText: 'Stage' })).toBeVisible();
+    });
+
+    await test.step('Run the folder containing both requests', async () => {
+      // Ensure we're in the correct collection context before selecting the folder
+      await expect(page.locator('#sidebar-collection-name', { hasText: 'collection' })).toBeVisible();
+
+      // Right-click on the folder to open context menu
+      await page.getByText('multiple-persist-vars-folder', { exact: true }).click({ button: 'right' });
+
+      // Click on Run option
+      await page.getByText('Run', { exact: true }).click();
+
+      // Click Run button in the modal
+      await page.getByRole('button', { name: 'Run', exact: true }).click();
+
+      // Wait for the folder to finish running
+      await page.getByRole('button', { name: 'Run Again' }).waitFor({ timeout: 30000 });
+    });
+
+    await test.step('Verify both environment variables are set in UI', async () => {
+      // Ensure we're still in the correct collection context
+      await expect(page.locator('#sidebar-collection-name', { hasText: 'collection' })).toBeVisible();
+
+      await page.getByTestId('environment-selector-trigger').click();
+      await page.locator('#configure-env').click();
+      await expect(page.getByRole('row', { name: 'multiple-persist-vars-key1' }).getByRole('cell').nth(1)).toBeVisible();
+      await expect(page.getByRole('row', { name: 'value1' }).getByRole('cell').nth(2)).toBeVisible();
+      await expect(page.getByRole('row', { name: 'multiple-persist-vars-key2' }).getByRole('cell').nth(1)).toBeVisible();
+      await expect(page.getByRole('row', { name: 'value2' }).getByRole('cell').nth(2)).toBeVisible();
+      await page.getByTestId('modal-close-button').click();
+    });
+
+    await test.step('Verify variables are persisted to file', async () => {
+      // Check that the variables are written to the Stage.bru file
+      const stageBruPath = path.join(__dirname, 'fixtures/collection/environments/Stage.bru');
+      const stageBruContent = fs.readFileSync(stageBruPath, 'utf8');
+
+      // Both variables should be present in the file
+      expect(stageBruContent).toContain('multiple-persist-vars-key1');
+      expect(stageBruContent).toContain('value1');
+      expect(stageBruContent).toContain('multiple-persist-vars-key2');
+      expect(stageBruContent).toContain('value2');
+    });
+  });
+});

tests/runner/collection-run-report/collection-run-report.spec.ts-snapshots/cli-junit-report-default-darwin.xml

@@ -0,0 +1,29 @@
+<?xml version="1.0"?>
+<testsuites>
+  <testsuite name="/test/path/collection" errors="0" failures="0" skipped="0" tests="4" timestamp="2024-01-01T00:00:00.000" hostname="test-host" time="0.100">
+    <testcase name="Status code is 200" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response is an object" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response has slideshow property" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Slideshow has title" status="pass" classname="/test/path/collection" time="0.100"/>
+  </testsuite>
+  <testsuite name="/test/path/collection" errors="0" failures="1" skipped="0" tests="5" timestamp="2024-01-01T00:00:00.000" hostname="test-host" time="0.100">
+    <testcase name="This test will fail" status="fail" classname="/test/path/collection" time="0.100">
+      <failure type="failure" message="expected 200 to equal 404"/>
+    </testcase>
+    <testcase name="Status code is 200" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response is an object" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response has uuid property" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="UUID is a string" status="pass" classname="/test/path/collection" time="0.100"/>
+  </testsuite>
+  <testsuite name="/test/path/collection" errors="0" failures="0" skipped="0" tests="3" timestamp="2024-01-01T00:00:00.000" hostname="test-host" time="0.100">
+    <testcase name="Status code is 200" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response has json field" status="pass" classname="/test/path/collection" time="0.100"/>
+    <testcase name="Response json has username" status="pass" classname="/test/path/collection" time="0.100"/>
+  </testsuite>
+  <testsuite name="/test/path/collection" errors="0" failures="1" skipped="0" tests="2" timestamp="2024-01-01T00:00:00.000" hostname="test-host" time="0.100">
+    <testcase name="This test will also fail" status="fail" classname="/test/path/collection" time="0.100">
+      <failure type="failure" message="expected 200 to equal 500"/>
+    </testcase>
+    <testcase name="Status code is 200" status="pass" classname="/test/path/collection" time="0.100"/>
+  </testsuite>
+</testsuites>