Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions tests/collection/open/open-collection.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import path from 'path';
import { test, expect } from '../../../playwright';
import {
buildCollectionLocators,
buildCommonLocators,
clickRemoveInCollectionMenu,
closeAllCollections,
confirmRemoveCollection,
createCollection,
openCollectionActionsMenu
} from '../../utils/page';

test.describe('Open collection sanity testcases', () => {
test('TC-2614: Verify user able to Remove the Opened collection from the sidebar', { tag: '@sanity' }, async ({ page, createTmpDir }) => {
const collectionName = 'remove-test-collection';
const collectionLocation = await createTmpDir(collectionName);
const collectionPath = path.join(collectionLocation, collectionName);
const locators = buildCommonLocators(page);
const collectionLocators = buildCollectionLocators(page);

await test.step('create collection', async () => {
await createCollection(page, collectionName, collectionLocation);
});

await test.step('open collection actions menu and verify Remove option is shown', async () => {
await openCollectionActionsMenu(page, collectionName);
await expect(locators.dropdown.item('Remove')).toBeVisible();
});

await test.step('click Remove and verify confirmation modal shows path and CTAs', async () => {
await clickRemoveInCollectionMenu(page);
await expect(collectionLocators.removeModal()).toBeVisible();
await expect(collectionLocators.removeButton()).toBeVisible();
await expect(collectionLocators.cancelButton()).toBeVisible();
await expect(collectionLocators.removeModalPath()).toContainText(collectionPath);
});

await test.step('confirm removal and verify success toast', async () => {
await confirmRemoveCollection(page);
await expect(collectionLocators.removedFromWorkspaceToast()).toBeVisible();
await expect(locators.sidebar.collection(collectionName)).not.toBeVisible();
});
});
});
83 changes: 50 additions & 33 deletions tests/collection/open/open-multiple-collections.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test.describe('Open Multiple Collections', () => {
});
});

test('Should open multiple collections using Open Collection feature', async ({
test('TC777: Verify opening of multiple collections from parent folder with successful launch', { tag: '@sanity' }, async ({
page,
electronApp,
createTmpDir
Expand All @@ -29,43 +29,60 @@ test.describe('Open Multiple Collections', () => {
const collection1Dir = await createTmpDir('collection-1');
const collection2Dir = await createTmpDir('collection-2');

// Create bruno.json for first collection
const collection1Config = {
version: '1',
name: 'Test Collection 1',
type: 'collection'
};
// Create bruno.json for second collection
const collection2Config = {
version: '1',
name: 'Test Collection 2',
type: 'collection'
};

fs.writeFileSync(path.join(collection1Dir, 'bruno.json'), JSON.stringify(collection1Config, null, 2));
fs.writeFileSync(path.join(collection2Dir, 'bruno.json'), JSON.stringify(collection2Config, null, 2));

// Mock the electron dialog to return multiple folder selections
await electronApp.evaluate(({ dialog }, { collection1Dir, collection2Dir }) => {
dialog.showOpenDialog = async () => ({
canceled: false,
filePaths: [collection1Dir, collection2Dir]
});
},
{ collection1Dir, collection2Dir });

await expect(page.locator('#sidebar-collection-name').getByText('Test Collection 1')).not.toBeVisible();
await test.step('Navigate to the parent folder containing multiple collections', async () => {
// Create bruno.json for first collection
const collection1Config = {
version: '1',
name: 'Test Collection 1',
type: 'collection'
};
// Create bruno.json for second collection
const collection2Config = {
version: '1',
name: 'Test Collection 2',
type: 'collection'
Comment on lines +34 to +43

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use per-test collection names here.

These fixed names make the text locators vulnerable to persisted/sidebar state from another test run, so this spec can bind to the wrong collection and pass for the wrong reason. Derive the names from the isolated temp dirs instead.

Minimal change
+      const collection1Name = path.basename(collection1Dir);
+      const collection2Name = path.basename(collection2Dir);
       const collection1Config = {
         version: '1',
-        name: 'Test Collection 1',
+        name: collection1Name,
         type: 'collection'
       };
       const collection2Config = {
         version: '1',
-        name: 'Test Collection 2',
+        name: collection2Name,
         type: 'collection'
       };
@@
-    const collection1Element = page.locator('`#sidebar-collection-name`').getByText('Test Collection 1');
-    const collection2Element = page.locator('`#sidebar-collection-name`').getByText('Test Collection 2');
+    const collection1Element = page.locator('`#sidebar-collection-name`').getByText(collection1Name);
+    const collection2Element = page.locator('`#sidebar-collection-name`').getByText(collection2Name);

As per coding guidelines, "E2E tests must be parallel-safe... Each test gets isolated temp paths and unique workspace/project names." As per path instructions, "Each test gets isolated temp paths and unique workspace/project names."

Also applies to: 61-62

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/collection/open/open-multiple-collections.spec.ts` around lines 34 -
43, The open-multiple-collections spec is using hardcoded collection names that
can collide with persisted sidebar state across runs. Update the test setup in
open-multiple-collections.spec.ts so the collection names created in the
collection1Config and collection2Config blocks are derived from each test’s
isolated temp directory or another per-test unique value, and make the same
change for the additional collection name usage referenced later in the spec.
Keep the identifiers stable within a single test but unique per test to ensure
the locators in this scenario bind to the correct collections.

Sources: Coding guidelines, Path instructions

};

fs.writeFileSync(path.join(collection1Dir, 'bruno.json'), JSON.stringify(collection1Config, null, 2));
fs.writeFileSync(path.join(collection2Dir, 'bruno.json'), JSON.stringify(collection2Config, null, 2));
});

// Click on plus icon button and then "Open collection" in the dropdown
await page.getByTestId('collections-header-add-menu').click();
await page.locator('.tippy-box .dropdown-item').filter({ hasText: 'Open collection' }).click();
await test.step('Select two different collections within the parent folder simultaneously', async () => {
// Mock the electron dialog to return multiple folder selections
await electronApp.evaluate(({ dialog }, { collection1Dir, collection2Dir }) => {
dialog.showOpenDialog = async () => ({
canceled: false,
filePaths: [collection1Dir, collection2Dir]
});
},
{ collection1Dir, collection2Dir });
});

// Wait for both collections to appear in the sidebar
const collection1Element = page.locator('#sidebar-collection-name').getByText('Test Collection 1');
const collection2Element = page.locator('#sidebar-collection-name').getByText('Test Collection 2');

await expect(collection1Element).toBeVisible();
await expect(collection2Element).toBeVisible();
await test.step('Initiate the simultaneous opening command', async () => {
await expect(collection1Element).not.toBeVisible();

// Click on plus icon button and then "Open collection" in the dropdown
await page.getByTestId('collections-header-add-menu').click();
await page.locator('.tippy-box .dropdown-item').filter({ hasText: 'Open collection' }).click();
});

await test.step('Verify the launch status of both collections', async () => {
await expect(collection1Element).toBeVisible();
await expect(collection2Element).toBeVisible();
});
Comment on lines +64 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Re-add the pre-open check for the second collection.

Right now leaked state for collection2Element goes unnoticed, and the later visibility assertion can still pass without proving the open action surfaced both collections.

As per coding guidelines, "Cover both the happy path and the realistically problematic paths." As per path instructions, "E2E tests must be parallel-safe. No shared user data directories... or global app state."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/collection/open/open-multiple-collections.spec.ts` around lines 64 -
75, The open-multiple-collections test is missing a pre-open visibility check
for collection2Element, so leaked state can slip through unnoticed. In
open-multiple-collections.spec.ts, update the test step around the open action
to assert both collection1Element and collection2Element are not visible before
clicking the Open collection flow, using the existing collection1Element and
collection2Element locators. Keep the post-action visibility assertions, but
make sure the test proves the command surfaced both collections from a clean
state.

Sources: Coding guidelines, Path instructions


await test.step('Check the functionality and accessibility of each opened collection', async () => {
await collection1Element.click();
await collection2Element.click();
});
Comment on lines +77 to +80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Assert a user-visible outcome after each click.

This step only proves the locators are clickable. It does not verify that either collection actually became active/opened. Please add an observable assertion after each click (for example, selected sidebar state or the active collection pane/header).

As per coding guidelines, "Tests must verify user-visible behaviour, not implementation details." As per path instructions, "Use multiple assertions."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/collection/open/open-multiple-collections.spec.ts` around lines 77 -
80, The open-multiple-collections test step only clicks collection locators
without proving either collection became active. Update the test around the
collection1Element and collection2Element clicks to add a user-visible assertion
after each click, such as verifying the selected sidebar state or the active
collection pane/header, so the test checks behavior not just clickability. Keep
the assertions in the same test.step and use multiple assertions to confirm both
opened collections are visibly active.

Sources: Coding guidelines, Path instructions


await test.step('Confirm no performance degradation or system resource overload', async () => {
await expect(page.getByTestId('collections')).toBeVisible();
await expect(page.getByTestId('collections-header-add-menu')).toBeEnabled();
});

// cleanup: close all collections
await closeAllCollections(page);
Expand Down
1 change: 0 additions & 1 deletion tests/utils/page/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ const closeAllCollections = async (page) => {
await expect(page.getByTestId('collections').locator('.collection-name')).toHaveCount(0);
});
};

/**
* Open a collection from the sidebar and accept the JavaScript Sandbox modal
* @param page - The page object
Expand Down
62 changes: 62 additions & 0 deletions tests/utils/page/collection.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { Page, expect, test } from '../../../playwright';
import { buildCommonLocators } from './locators';

/**
* Builds locators for collection sidebar actions (remove modal, success toast).
*/
export const buildCollectionLocators = (page: Page) => {
const removeModal = () => page.locator('.bruno-modal').filter({ hasText: 'Remove Collection' });

return {
removeModal,
removeModalPath: () => removeModal().locator('.collection-path'),
removeButton: () => removeModal().getByRole('button', { name: 'Remove', exact: true }),
cancelButton: () => removeModal().getByRole('button', { name: 'Cancel', exact: true }),
discardAllAndRemoveButton: () => page.getByRole('button', { name: 'Discard All and Remove' }),
removedFromWorkspaceToast: () => page.getByText('Collection removed from workspace')
};
};

/**
* Hover a collection row and open its actions (three-dots) menu.
*/
export const openCollectionActionsMenu = async (page: Page, collectionName: string) => {
await test.step(`Open actions menu for collection "${collectionName}"`, async () => {
const locators = buildCommonLocators(page);
await locators.sidebar.collectionRow(collectionName).hover();
await locators.actions.collectionActions(collectionName).click();
});
};

/**
* Click Remove in the open collection actions menu and wait for the confirmation modal.
*/
export const clickRemoveInCollectionMenu = async (page: Page) => {
const locators = buildCommonLocators(page);
const collectionLocators = buildCollectionLocators(page);
await locators.dropdown.item('Remove').click();
await collectionLocators.removeModal().waitFor({ state: 'visible', timeout: 5000 });
};

/**
* Confirm removal in the Remove Collection modal (handles drafts modal when present).
*/
export const confirmRemoveCollection = async (
page: Page,
options: { forceDiscard?: boolean } = {}
) => {
await test.step('Confirm Remove Collection', async () => {
const collectionLocators = buildCollectionLocators(page);
const hasDiscardButton = await collectionLocators.discardAllAndRemoveButton().isVisible().catch(() => false);

if (hasDiscardButton) {
await collectionLocators.discardAllAndRemoveButton().click(
options.forceDiscard ? { force: true } : undefined
);
} else {
await collectionLocators.removeButton().click();
}

await collectionLocators.removeModal().waitFor({ state: 'hidden', timeout: 5000 });
Comment on lines +48 to +60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Click Remove before probing for the discard dialog.

Discard All and Remove is a follow-up confirmation, so checking for it before removeButton().click() means dirty collections still fall through the normal branch and never complete the discard step. This helper therefore does not actually handle the draft-confirmation flow it advertises.

Proposed fix
 export const confirmRemoveCollection = async (
   page: Page,
   options: { forceDiscard?: boolean } = {}
 ) => {
   await test.step('Confirm Remove Collection', async () => {
     const collectionLocators = buildCollectionLocators(page);
-    const hasDiscardButton = await collectionLocators.discardAllAndRemoveButton().isVisible().catch(() => false);
-
-    if (hasDiscardButton) {
-      await collectionLocators.discardAllAndRemoveButton().click(
-        options.forceDiscard ? { force: true } : undefined
-      );
-    } else {
-      await collectionLocators.removeButton().click();
-    }
+    await collectionLocators.removeButton().click();
+
+    const discardButton = collectionLocators.discardAllAndRemoveButton();
+    const needsDiscardConfirm = await discardButton
+      .waitFor({ state: 'visible', timeout: 1000 })
+      .then(() => true)
+      .catch(() => false);
+
+    if (needsDiscardConfirm) {
+      await discardButton.click(options.forceDiscard ? { force: true } : undefined);
+    }
 
     await collectionLocators.removeModal().waitFor({ state: 'hidden', timeout: 5000 });
   });
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await test.step('Confirm Remove Collection', async () => {
const collectionLocators = buildCollectionLocators(page);
const hasDiscardButton = await collectionLocators.discardAllAndRemoveButton().isVisible().catch(() => false);
if (hasDiscardButton) {
await collectionLocators.discardAllAndRemoveButton().click(
options.forceDiscard ? { force: true } : undefined
);
} else {
await collectionLocators.removeButton().click();
}
await collectionLocators.removeModal().waitFor({ state: 'hidden', timeout: 5000 });
await test.step('Confirm Remove Collection', async () => {
const collectionLocators = buildCollectionLocators(page);
await collectionLocators.removeButton().click();
const discardButton = collectionLocators.discardAllAndRemoveButton();
const needsDiscardConfirm = await discardButton
.waitFor({ state: 'visible', timeout: 1000 })
.then(() => true)
.catch(() => false);
if (needsDiscardConfirm) {
await discardButton.click(options.forceDiscard ? { force: true } : undefined);
}
await collectionLocators.removeModal().waitFor({ state: 'hidden', timeout: 5000 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/utils/page/collection.ts` around lines 48 - 60, The Confirm Remove
Collection helper is checking for discard-first instead of triggering the
initial remove action, so the draft-confirmation flow is never reached. Update
buildCollectionLocators usage in Confirm Remove Collection to click
removeButton() first, then detect and handle discardAllAndRemoveButton() if it
appears, using options.forceDiscard only for that follow-up click before waiting
on removeModal().hide.

});
};
1 change: 1 addition & 0 deletions tests/utils/page/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './actions';
export * from './runner';
export * from './locators';
export * from './collection';
export * from './mounting';
export * from '../snapshot';
Loading