From b552cd92fa1d8a4c430d37d1daffc16653c433eb Mon Sep 17 00:00:00 2001 From: Bob Qiu Date: Mon, 19 Oct 2026 17:46:30 +0800 Subject: [PATCH] Add tests and enums for handling undiscardable changes in Git sync - Introduced `GitChangeList` enum to categorize changes as Staged or Unstaged. - Implemented multiple test cases to verify the behavior of undiscardable changes: - Committing a reordered file leaves the working tree clean. - Confirming discard on a reordered file brings the change back. - Mixed changes (real edits with reorders) are not flagged until the real edit is discarded. - Non-Insomnia added and deleted files show no undiscardable warning. - Files from older schemas are flagged as undiscardable. - Real edits show no undiscardable warning in diffs or discard confirmations. - Staged hand-reordered files show no undiscardable warning. - Warnings are consistent in both diffs and discard confirmations for hand-reordered files. --- .claude/skills/write-insomnia-test/SKILL.md | 2 +- .../skills/write-insomnia-test/reference.md | 24 ++ .../insomnia-test/enums/git-change-list.ts | 4 + packages/insomnia-test/flows/git-sync.flow.ts | 16 + packages/insomnia-test/pages/git-sync.page.ts | 371 ++++++++++-------- .../commit-clears-change.spec.ts | 44 +++ ...firming-discard-brings-change-back.spec.ts | 35 ++ .../git-sync/undiscardable-change/helpers.ts | 58 +++ .../mixed-change-is-not-flagged.spec.ts | 47 +++ ...-and-deleted-files-show-no-warning.spec.ts | 62 +++ .../old-schema-file-flagged.spec.ts | 34 ++ .../real-edit-shows-no-warning.spec.ts | 42 ++ .../staged-file-shows-no-warning.spec.ts | 30 ++ ...g-in-diff-and-discard-confirmation.spec.ts | 36 ++ 14 files changed, 636 insertions(+), 169 deletions(-) create mode 100644 packages/insomnia-test/enums/git-change-list.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/commit-clears-change.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/confirming-discard-brings-change-back.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/helpers.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/mixed-change-is-not-flagged.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/non-insomnia-added-and-deleted-files-show-no-warning.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/old-schema-file-flagged.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/real-edit-shows-no-warning.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/staged-file-shows-no-warning.spec.ts create mode 100644 packages/insomnia-test/tests/git-sync/undiscardable-change/warning-in-diff-and-discard-confirmation.spec.ts diff --git a/.claude/skills/write-insomnia-test/SKILL.md b/.claude/skills/write-insomnia-test/SKILL.md index 21ceb46213..cb657129e6 100644 --- a/.claude/skills/write-insomnia-test/SKILL.md +++ b/.claude/skills/write-insomnia-test/SKILL.md @@ -127,7 +127,7 @@ Skip this step when nothing about the real app's live behavior is in question. - This step only applies once step 2 has concluded the scenario needs a genuinely new file — don't re-decide placement for something you're extending in place. - Main path (create → act → assert, the "everything works" case): `tests//happy-path.spec.ts`. If one already exists for that domain, do not overwrite it with a different scenario — add a new, separately named file instead (a same-intent extension belongs under step 2, not here). - Edge case / error scenario: `tests//.spec.ts` — name the file after the scenario itself (e.g. `unlinked-environment-variable.spec.ts`), never `test1.spec.ts` or similar. -- **A family of related specs for one feature/ticket** (several edge-case files that all drive the same scripted mock server, e.g. one per case of a test plan) may live in a subdirectory of the domain (`tests/mcp-client/error-response/`) — one spec file per case, named after the scenario. Shared setup (a `connectTo(user, config)`-style helper, canned payloads, expected-title helpers) goes in a non-spec `helpers.ts` next to them (it isn't matched by the `*.spec.ts` test pattern); adjust relative imports by one extra `../`. Don't use this for a single spec — that stays a plain file under `tests//`. +- **A family of related specs for one feature/ticket** (several edge-case files that all drive the same scripted mock server, e.g. one per case of a test plan) may live in a subdirectory of the domain (`tests/mcp-client/error-response/`, `tests/git-sync/undiscardable-change/`) — one spec file per case, named after the scenario. Shared setup (a `connectTo(user, config)`-style helper, canned payloads, expected-title helpers) goes in a non-spec `helpers.ts` next to them (it isn't matched by the `*.spec.ts` test pattern); adjust relative imports by one extra `../`. Don't use this for a single spec — that stays a plain file under `tests//`. - `tests/data/import/` and `tests/data/export/` sit together under `tests/data/` since both drive Preferences -> Data; `tests/data/import/` is further split by source: `file/`, `url/`, `curl/`, `clipboard/`, `mcp/`. - **If the test needs a fixture file** (a JSON/YAML/WSDL/etc. imported or read via `path.join(__dirname, "...")`), give the spec its own directory named after the spec file (without `.spec.ts`), and put the spec and every fixture it uses inside that directory together — e.g. `tests/data/import/file/import-open-api/import-open-api.spec.ts` + `tests/data/import/file/import-open-api/httpbingo-get.json`. Adjust the relative import depth to `misc/fixtures`/`enums/`/`models/` by one extra `../` versus a plain domain-level spec, since the file now sits one directory deeper. A spec with no fixture stays a plain file directly under `tests//` (or `tests/data/import//`) — don't create a directory for it. diff --git a/.claude/skills/write-insomnia-test/reference.md b/.claude/skills/write-insomnia-test/reference.md index 6fa28ad8cb..627e7c7490 100644 --- a/.claude/skills/write-insomnia-test/reference.md +++ b/.claude/skills/write-insomnia-test/reference.md @@ -1232,6 +1232,7 @@ getChanges(): Promise<{ staged: string[]; unstaged: string[] }> // file names ( stageChanges(fileNames: string[]): Promise // stages each file individually from the Commit modal; fileNames as listed, e.g. "foo.yaml" unstageChanges(fileNames: string[]): Promise // unstages each file individually discardAllChanges(): Promise // discards every uncommitted local change +discardChangeAfterReview(fileName: string): Promise // opens the file's diff, clicks Discard, confirms, closes the modal — does NOT wait for the row to leave the unstaged list, so it works for a change that comes straight back (see "Undiscardable change" below) createBranch(branch: string): Promise // creates + checks out a new local branch createBranchExpectingError(branch: string): Promise // submits a duplicate/invalid branch name, returns the validation error text under "New branch name", closes the modal getBranches(): Promise<{ local: string[]; remote: string[] }> // local + remote-only branch lists from the Branches modal, then closes it @@ -1246,6 +1247,29 @@ deleteBranch(branch: string, switchToBranch = "master"): Promise // check **Direct `gitSyncPage` calls with no Flow wrapper** (the real specs use these — destructure `gitSyncPage` from `user.pageManager`): `openCommitDialog()`/`closeCommitDialog()`/`isCommitDialogOpen()`, `commitStaged(message?)` (fills the Message field, **clearing it when omitted** since it keeps its text between attempts), `getStagedChanges()`/`getUnstagedChanges()`, `stageChange(fileName)`/`unstageChange(fileName)`, `openHistory()`/`getLatestCommit()`/`closeHistory()`, `getCurrentBranch()`. The "Stage All Changes" prompt — shown when Commit is clicked with **nothing staged** — has `getStageAllPromptText()` (waits for it; text contains "There are no staged changes to commit"), `cancelStageAllPrompt()` (leaves the modal open, nothing committed), and `confirmStageAllPrompt()` ("Yes": stages everything + commits, waits for the modal to close). `isCommitDisabled()` was removed — the Commit button is no longer disabled with nothing staged, it opens this prompt instead. `openCommitDialog()` now also waits for at least one change row, so reading staged/unstaged right after it can't see a still-empty list. +**Undiscardable-change warning (INS-3830) — direct `gitSyncPage` calls, no Flow wrapper** (specs in `tests/git-sync/undiscardable-change/`, a family directory with a shared `helpers.ts`). All of these need `openCommitDialog()` first and leave the dialog open unless noted; `closeCommitDialog()` is only needed before the next `openCommitDialog()`/menu use, not at the end of a test: + +```ts +// enums/git-change-list.ts — GitChangeList.Staged = "Staged changes" | GitChangeList.Unstaged = "Unstaged changes" +openChangeDiff(list: GitChangeList, fileName: string): Promise // clicks the row and waits for the diff pane (Stage/Unstage this file button); the warning flag arrives with the diff, so once this resolves its presence/absence is settled +isUndiscardableWarningVisibleInDiff(): Promise // "This change is only Insomnia internally reformatting the file…" banner above the diff +openDiscardConfirmation(fileName: string): Promise // clicks Discard on an unstaged row, waits for the "Discard Changes" dialog, does not confirm +getDiscardConfirmationText(): Promise // the dialog's text +isUndiscardableWarningVisibleInDiscardDialog(): Promise +confirmDiscard(): Promise // confirms without waiting for the row to disappear (unlike discardChange(), which does) +cancelDiscard(): Promise +hasUncommittedChanges(): Promise // reads the Git Sync menu's Commit item (disabled = clean tree); opens/closes the menu itself, no Commit dialog needed. Poll it — it lags the watcher +``` + +Always pass `GitChangeList.Staged`/`.Unstaged` (never the raw "Staged changes" string) to `openChangeDiff`, and note `getChanges()`/`openCommitDialog()` **throw/time out on a clean tree** (the Commit item is disabled), so never use them to assert "nothing changed" — use `hasUncommittedChanges()`. + +**Facts learned building these specs:** +- The Commit modal only lists **`.yaml`** files (`git-vcs.ts` filters on extension). A "non-Insomnia file" must be a non-Insomnia `.yaml` (e.g. `openapi: 3.0.0…`), never README.md. +- Seed a remote file **before** creating the Git project: `pushServerFile({ path, content })` then `workspaceFlow.create(new Project(…, ProjectType.Git), GIT_CREDENTIAL.name)` — the clone imports it and the watcher re-exports it, so a non-canonical file shows up as an unstaged change with no `pull()` needed. +- A hand-reordered / old-schema Insomnia file is built by `buildReorderedCollectionFile(name, { oldSchema? })` in `tests/git-sync/undiscardable-change/helpers.ts` (folders listed against their `created` order; `oldSchema` drops `schema_version` and adds a header `id`). Copy that helper's shape for any other "file Insomnia rewrites" fixture; `buildCollectionFile()` from git-fixtures is already canonical-ish and won't trigger it. +- To modify a file on disk (e.g. a non-Insomnia yaml), pass `{ cloneParentDir: fs.mkdtempSync(path.join(os.tmpdir(), "…")) }` as the 3rd arg of the Git `workspaceFlow.create()`; the clone is the single sub-directory inside it (`path.join(dir, fs.readdirSync(dir)[0])`). Added/deleted files are easiest via the UI (`workspaceFlow.create(project, collection, fileName)` / `workspaceFlow.delete(collection)`). +- After a discard of an undiscardable file the change reappears asynchronously — poll `getChanges().unstaged` for it rather than reading once. + `Commit` (`models/commit.ts`): `{ message: string; id?: string }` — positional constructor (`new Commit(message)`), `id` populated after `commit()`/`commitAndPush()` read it back from the History modal's row `data-key` (the real git OID). **Creating a Git Sync project and its collections** goes through `workspaceFlow.create()`'s dedicated overloads, not a separate flow: diff --git a/packages/insomnia-test/enums/git-change-list.ts b/packages/insomnia-test/enums/git-change-list.ts new file mode 100644 index 0000000000..f8db778cb1 --- /dev/null +++ b/packages/insomnia-test/enums/git-change-list.ts @@ -0,0 +1,4 @@ +export enum GitChangeList { + Staged = "Staged changes", + Unstaged = "Unstaged changes", +} diff --git a/packages/insomnia-test/flows/git-sync.flow.ts b/packages/insomnia-test/flows/git-sync.flow.ts index c73d2c36ac..68c4a37742 100644 --- a/packages/insomnia-test/flows/git-sync.flow.ts +++ b/packages/insomnia-test/flows/git-sync.flow.ts @@ -1,3 +1,4 @@ +import { GitChangeList } from "../enums/git-change-list"; import { Commit } from "../models/commit"; import { BaseFlow } from "./base.flow"; @@ -127,6 +128,21 @@ export class GitSyncFlow extends BaseFlow { ); } + /** + * Opens a file's diff and discards it, confirming the dialog, without + * waiting for the file to leave the unstaged list (so it also works for a + * change that comes straight back), then closes the modal. + * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" + */ + async discardChangeAfterReview(fileName: string): Promise { + const { gitSyncPage } = this.pageManager; + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + await gitSyncPage.openDiscardConfirmation(fileName); + await gitSyncPage.confirmDiscard(); + await gitSyncPage.closeCommitDialog(); + } + /** * Discards every uncommitted local change. */ diff --git a/packages/insomnia-test/pages/git-sync.page.ts b/packages/insomnia-test/pages/git-sync.page.ts index 300ff0b082..e9ba472159 100644 --- a/packages/insomnia-test/pages/git-sync.page.ts +++ b/packages/insomnia-test/pages/git-sync.page.ts @@ -1,35 +1,38 @@ -import { expect } from "@playwright/test"; +import { expect } from '@playwright/test'; -import { DEFAULT_TIMEOUT } from "../misc/constants"; -import { BasePage } from "./base.page"; +import { GitChangeList } from '../enums/git-change-list'; +import { DEFAULT_TIMEOUT } from '../misc/constants'; +import { BasePage } from './base.page'; export class GitSyncPage extends BasePage { - private readonly syncDropdownButton = this.page.getByTestId("git-dropdown"); - private readonly syncMenu = this.page.getByRole("menu", { - name: "Git Sync", + private readonly syncDropdownButton = this.page.getByTestId('git-dropdown'); + private readonly syncMenu = this.page.getByRole('menu', { + name: 'Git Sync', }); - private readonly commitDialog = this.page.getByRole("dialog", { - name: "Commit Changes", + private readonly commitDialog = this.page.getByRole('dialog', { + name: 'Commit Changes', }); - private readonly branchesDialog = this.page.getByRole("dialog", { - name: "Branches", + private readonly branchesDialog = this.page.getByRole('dialog', { + name: 'Branches', }); - private readonly stageAllPrompt = this.page - .getByRole("dialog") - .filter({ hasText: "Stage All Changes" }); - private readonly mergeConflictsDialog = this.page.getByRole("dialog", { - name: "Resolve conflicts", + private readonly stageAllPrompt = this.page.getByRole('dialog').filter({ hasText: 'Stage All Changes' }); + private readonly mergeConflictsDialog = this.page.getByRole('dialog', { + name: 'Resolve conflicts', }); private readonly historyDialog = this.page - .getByRole("dialog") - .filter({ has: this.page.getByRole("heading", { name: "History" }) }); + .getByRole('dialog') + .filter({ has: this.page.getByRole('heading', { name: 'History' }) }); - private readonly changeRow = ( - list: "Staged changes" | "Unstaged changes", - fileName: string, - ) => + private readonly discardDialog = this.page + .getByRole('dialog') + .filter({ has: this.page.getByRole('heading', { name: 'Discard Changes' }) }); + private readonly undiscardableWarning = this.page.getByText( + 'This change is only Insomnia internally reformatting the file', + ); + + private readonly changeRow = (list: GitChangeList, fileName: string) => this.commitDialog - .getByRole("grid", { name: list, exact: true }) + .getByRole('grid', { name: list, exact: true }) .locator('[role="row"][data-key]') .filter({ hasText: fileName }); @@ -43,9 +46,9 @@ export class GitSyncPage extends BasePage { */ async navigate(): Promise { await this.page - .getByText("Setup Git Credentials") + .getByText('Setup Git Credentials') .or(this.syncDropdownButton) - .waitFor({ state: "visible", timeout: DEFAULT_TIMEOUT }); + .waitFor({ state: 'visible', timeout: DEFAULT_TIMEOUT }); } /** @@ -65,9 +68,7 @@ export class GitSyncPage extends BasePage { */ async switchBranch(branch: string): Promise { await this.openSyncMenu(); - await this.syncMenu - .getByRole("menuitemradio", { name: branch, exact: true }) - .click(); + await this.syncMenu.getByRole('menuitemradio', { name: branch, exact: true }).click(); await expect(this.syncDropdownButton).toContainText(branch, { timeout: DEFAULT_TIMEOUT, }); @@ -78,9 +79,7 @@ export class GitSyncPage extends BasePage { */ async fetch(): Promise { await this.openSyncMenu(); - await this.syncMenu - .getByRole("menuitemradio", { name: "Fetch", exact: true }) - .click(); + await this.syncMenu.getByRole('menuitemradio', { name: 'Fetch', exact: true }).click(); } /** @@ -90,9 +89,7 @@ export class GitSyncPage extends BasePage { */ async pull(): Promise { await this.openSyncMenu(); - await this.syncMenu - .getByRole("menuitemradio", { name: "Pull", exact: true }) - .click(); + await this.syncMenu.getByRole('menuitemradio', { name: 'Pull', exact: true }).click(); } /** @@ -101,10 +98,8 @@ export class GitSyncPage extends BasePage { */ async push(): Promise { await this.openSyncMenu(); - await this.syncMenu - .getByRole("menuitemradio", { name: "Push", exact: true }) - .click(); - await expect(this.page.getByText("Push completed")).toBeVisible({ + await this.syncMenu.getByRole('menuitemradio', { name: 'Push', exact: true }).click(); + await expect(this.page.getByText('Push completed')).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); } @@ -126,29 +121,23 @@ export class GitSyncPage extends BasePage { async openCommitDialog(): Promise { await expect(async () => { await this.openSyncMenu(); - const commitItem = this.syncMenu.getByRole("menuitemradio", { - name: "Commit", + const commitItem = this.syncMenu.getByRole('menuitemradio', { + name: 'Commit', exact: true, }); try { await expect(commitItem).toBeEnabled({ timeout: 3000 }); } catch (error) { - await this.page.keyboard.press("Escape"); + await this.page.keyboard.press('Escape'); throw error; } await commitItem.click(); }).toPass({ timeout: DEFAULT_TIMEOUT * 3 }); await expect(this.commitDialog).toBeVisible({ timeout: DEFAULT_TIMEOUT }); - await expect(this.commitDialog).not.toHaveAttribute( - "data-loading", - "true", - { - timeout: DEFAULT_TIMEOUT, - }, - ); - await expect( - this.commitDialog.locator('[role="row"][data-key]').first(), - ).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + await expect(this.commitDialog).not.toHaveAttribute('data-loading', 'true', { + timeout: DEFAULT_TIMEOUT, + }); + await expect(this.commitDialog.locator('[role="row"][data-key]').first()).toBeVisible({ timeout: DEFAULT_TIMEOUT }); } /** @@ -163,14 +152,14 @@ export class GitSyncPage extends BasePage { */ private async stageAllChanges(): Promise { await this.commitDialog - .getByRole("grid", { name: "Unstaged changes" }) - .getByRole("row") + .getByRole('grid', { name: 'Unstaged changes' }) + .getByRole('row') .first() - .waitFor({ state: "visible", timeout: DEFAULT_TIMEOUT }); + .waitFor({ state: 'visible', timeout: DEFAULT_TIMEOUT }); await this.commitDialog.locator('button[name="Stage all changes"]').click(); - await expect( - this.commitDialog.getByRole("heading", { name: /^Staged changes [1-9]/ }), - ).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + await expect(this.commitDialog.getByRole('heading', { name: /^Staged changes [1-9]/ })).toBeVisible({ + timeout: DEFAULT_TIMEOUT, + }); } /** @@ -180,12 +169,8 @@ export class GitSyncPage extends BasePage { */ async commit(message: string): Promise { await this.stageAllChanges(); - await this.commitDialog - .getByRole("textbox", { name: "Message" }) - .fill(message); - await this.commitDialog - .getByRole("button", { name: "Commit", exact: true }) - .click(); + await this.commitDialog.getByRole('textbox', { name: 'Message' }).fill(message); + await this.commitDialog.getByRole('button', { name: 'Commit', exact: true }).click(); await expect(this.commitDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } @@ -200,16 +185,10 @@ export class GitSyncPage extends BasePage { */ async commitAndPush(message: string): Promise { await this.stageAllChanges(); - await this.commitDialog - .getByRole("textbox", { name: "Message" }) - .fill(message); - await this.commitDialog - .getByRole("button", { name: "Commit and push" }) - .click(); + await this.commitDialog.getByRole('textbox', { name: 'Message' }).fill(message); + await this.commitDialog.getByRole('button', { name: 'Commit and push' }).click(); await expect(this.commitDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); - await expect( - this.page.getByText("Changes committed and pushed"), - ).toBeVisible({ + await expect(this.page.getByText('Changes committed and pushed')).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); } @@ -236,7 +215,7 @@ export class GitSyncPage extends BasePage { * @returns The staged file names */ async getStagedChanges(): Promise { - return this.readChangeRows("Staged changes"); + return this.readChangeRows(GitChangeList.Staged); } /** @@ -245,7 +224,7 @@ export class GitSyncPage extends BasePage { * @returns The unstaged file names */ async getUnstagedChanges(): Promise { - return this.readChangeRows("Unstaged changes"); + return this.readChangeRows(GitChangeList.Unstaged); } /** @@ -255,10 +234,10 @@ export class GitSyncPage extends BasePage { * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" */ async stageChange(fileName: string): Promise { - const row = this.changeRow("Unstaged changes", fileName); + const row = this.changeRow(GitChangeList.Unstaged, fileName); await row.hover(); await row.locator('button[name="Stage change"]').click(); - await expect(this.changeRow("Staged changes", fileName)).toBeVisible({ + await expect(this.changeRow(GitChangeList.Staged, fileName)).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); } @@ -270,10 +249,10 @@ export class GitSyncPage extends BasePage { * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" */ async unstageChange(fileName: string): Promise { - const row = this.changeRow("Staged changes", fileName); + const row = this.changeRow(GitChangeList.Staged, fileName); await row.hover(); await row.locator('button[name="Unstage change"]').click(); - await expect(this.changeRow("Unstaged changes", fileName)).toBeVisible({ + await expect(this.changeRow(GitChangeList.Unstaged, fileName)).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); } @@ -285,9 +264,9 @@ export class GitSyncPage extends BasePage { */ async unstageAllChanges(): Promise { await this.commitDialog.locator('button[name="Unstage all changes"]').click(); - await expect( - this.commitDialog.getByRole("heading", { name: /^Staged changes 0/ }), - ).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + await expect(this.commitDialog.getByRole('heading', { name: /^Staged changes 0/ })).toBeVisible({ + timeout: DEFAULT_TIMEOUT, + }); } /** @@ -297,13 +276,99 @@ export class GitSyncPage extends BasePage { * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" */ async discardChange(fileName: string): Promise { - const row = this.changeRow("Unstaged changes", fileName); + const row = this.changeRow(GitChangeList.Unstaged, fileName); await row.hover(); await row.locator('button[name="Discard change"]').click(); - await this.page.getByTestId("discard-changes-confirm-button").click(); + await this.page.getByTestId('discard-changes-confirm-button').click(); await expect(row).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } + /** + * Clicks a file's row in the open Commit modal to open its diff, and + * waits for the diff pane (its Stage/Unstage this-file button) to render + * — the undiscardable flag arrives with the same loader data as the + * diff, so once the pane is up the warning's presence is settled. + * Requires `openCommitDialog()` to have been called first. + * @param list - Which list the file is in + * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" + */ + async openChangeDiff(list: GitChangeList, fileName: string): Promise { + await this.changeRow(list, fileName).click(); + await expect( + this.commitDialog.getByRole('button', { + name: list === GitChangeList.Staged ? 'Unstage this file' : 'Stage this file', + }), + ).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + } + + /** + * Reads whether the open Commit modal's diff pane shows the "can't be + * discarded" warning. Requires `openChangeDiff()` to have been called. + * @returns True if the warning banner is visible in the diff pane + */ + async isUndiscardableWarningVisible(): Promise { + return this.commitDialog.getByText('This change is only Insomnia internally reformatting the file').isVisible(); + } + + /** + * Clicks Discard on a file's row and waits for the confirmation dialog, + * without confirming. Requires `openCommitDialog()` to have been called. + * @param fileName - The file name as listed in the modal, e.g. "foo.yaml" + */ + async openDiscardConfirmation(fileName: string): Promise { + const row = this.changeRow(GitChangeList.Unstaged, fileName); + await row.hover(); + await row.locator('button[name="Discard change"]').click(); + await expect(this.discardDialog).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + } + + /** + * Reads the open Discard confirmation dialog's body text. + * @returns The dialog's text content + */ + async getDiscardConfirmationText(): Promise { + return (await this.discardDialog.innerText()).trim(); + } + + /** + * Reads whether the open Discard confirmation dialog shows the "can't be + * discarded" warning. + * @returns True if the warning is visible in the dialog + */ + async isUndiscardableWarningVisibleInDiscardDialog(): Promise { + return this.discardDialog.getByText('This change is only Insomnia internally reformatting the file').isVisible(); + } + + /** + * Confirms the open Discard dialog without waiting for the file to leave + * the unstaged list (a file whose change can't be discarded comes back). + */ + async confirmDiscard(): Promise { + await this.page.getByTestId('discard-changes-confirm-button').click(); + await expect(this.discardDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); + } + + /** + * Cancels the open Discard confirmation dialog. + */ + async cancelDiscard(): Promise { + await this.discardDialog.getByRole('button', { name: 'Cancel' }).click(); + await expect(this.discardDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); + } + + /** + * Reads whether the Git Sync menu's "Commit" item is enabled, i.e. + * whether anything is uncommitted. Opens and closes the menu. + * @returns True if there are uncommitted changes + */ + async hasUncommittedChanges(): Promise { + await this.openSyncMenu(); + const enabled = await this.syncMenu.getByRole('menuitemradio', { name: 'Commit', exact: true }).isEnabled(); + await this.page.keyboard.press('Escape'); + await expect(this.syncMenu).toBeHidden({ timeout: DEFAULT_TIMEOUT }); + return enabled; + } + /** * Fills `message` (clearing the field when omitted, since it keeps its * previous text between attempts) and clicks "Commit" on whatever is @@ -314,12 +379,8 @@ export class GitSyncPage extends BasePage { * @param message - The commit message; omit to submit with an empty message */ async commitStaged(message?: string): Promise { - await this.commitDialog - .getByRole("textbox", { name: "Message" }) - .fill(message ?? ""); - await this.commitDialog - .getByRole("button", { name: "Commit", exact: true }) - .click(); + await this.commitDialog.getByRole('textbox', { name: 'Message' }).fill(message ?? ''); + await this.commitDialog.getByRole('button', { name: 'Commit', exact: true }).click(); } /** @@ -337,7 +398,7 @@ export class GitSyncPage extends BasePage { */ async getStageAllPromptText(): Promise { await expect(this.stageAllPrompt).toBeVisible({ timeout: DEFAULT_TIMEOUT }); - return (await this.stageAllPrompt.innerText()).replaceAll("\n", " "); + return (await this.stageAllPrompt.innerText()).replaceAll('\n', ' '); } /** @@ -345,7 +406,7 @@ export class GitSyncPage extends BasePage { * Commit modal open and nothing committed. */ async cancelStageAllPrompt(): Promise { - await this.stageAllPrompt.getByRole("button", { name: "Cancel" }).click(); + await this.stageAllPrompt.getByRole('button', { name: 'Cancel' }).click(); await expect(this.stageAllPrompt).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } @@ -354,7 +415,7 @@ export class GitSyncPage extends BasePage { * change and commits it, then waits for the Commit modal to close. */ async confirmStageAllPrompt(): Promise { - await this.stageAllPrompt.getByRole("button", { name: "Yes" }).click(); + await this.stageAllPrompt.getByRole('button', { name: 'Yes' }).click(); await expect(this.commitDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } @@ -362,9 +423,9 @@ export class GitSyncPage extends BasePage { * Waits until the Commit modal's staged list holds no files. */ async waitForNothingStaged(): Promise { - await expect( - this.commitDialog.getByRole("heading", { name: /^Staged changes 0/ }), - ).toBeVisible({ timeout: DEFAULT_TIMEOUT }); + await expect(this.commitDialog.getByRole('heading', { name: /^Staged changes 0/ })).toBeVisible({ + timeout: DEFAULT_TIMEOUT, + }); } /** @@ -375,14 +436,12 @@ export class GitSyncPage extends BasePage { */ async discardAllChanges(): Promise { await this.commitDialog - .getByRole("grid", { name: "Unstaged changes" }) - .getByRole("row") + .getByRole('grid', { name: 'Unstaged changes' }) + .getByRole('row') .first() - .waitFor({ state: "visible", timeout: DEFAULT_TIMEOUT }); - await this.commitDialog - .locator('button[name="Discard all changes"]') - .click(); - await this.page.getByTestId("discard-changes-confirm-button").click(); + .waitFor({ state: 'visible', timeout: DEFAULT_TIMEOUT }); + await this.commitDialog.locator('button[name="Discard all changes"]').click(); + await this.page.getByTestId('discard-changes-confirm-button').click(); await expect(this.commitDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } @@ -397,7 +456,7 @@ export class GitSyncPage extends BasePage { */ async openHistory(): Promise { await this.openSyncMenu(); - await this.syncMenu.getByText("History", { exact: true }).click(); + await this.syncMenu.getByText('History', { exact: true }).click(); await expect(this.historyDialog).toBeVisible({ timeout: DEFAULT_TIMEOUT }); } @@ -417,18 +476,16 @@ export class GitSyncPage extends BasePage { * for the log to finish loading. * @returns The latest commit's id and message, or undefined if the log is empty */ - async getLatestCommit(): Promise< - { id: string; message: string } | undefined - > { + async getLatestCommit(): Promise<{ id: string; message: string } | undefined> { const row = this.historyDialog.locator('[role="row"][data-key]').first(); try { await expect(row).toBeVisible({ timeout: DEFAULT_TIMEOUT }); } catch { return undefined; } - const id = await row.getAttribute("data-key"); + const id = await row.getAttribute('data-key'); if (!id) return undefined; - const message = await row.getByRole("rowheader").innerText(); + const message = await row.getByRole('rowheader').innerText(); return { id, message }; } @@ -436,7 +493,7 @@ export class GitSyncPage extends BasePage { * Closes the History modal via Escape. */ async closeHistory(): Promise { - await this.page.keyboard.press("Escape"); + await this.page.keyboard.press('Escape'); await expect(this.historyDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT }); } @@ -445,9 +502,7 @@ export class GitSyncPage extends BasePage { */ async openBranchesDialog(): Promise { await this.openSyncMenu(); - await this.syncMenu - .getByRole("menuitemradio", { name: "Branches", exact: true }) - .click(); + await this.syncMenu.getByRole('menuitemradio', { name: 'Branches', exact: true }).click(); await expect(this.branchesDialog).toBeVisible({ timeout: DEFAULT_TIMEOUT }); } @@ -460,12 +515,8 @@ export class GitSyncPage extends BasePage { * @param branch - The new branch's name */ async createBranch(branch: string): Promise { - await this.branchesDialog - .getByRole("textbox", { name: "New branch name:" }) - .fill(branch); - await this.branchesDialog - .getByRole("button", { name: "Create", exact: true }) - .click(); + await this.branchesDialog.getByRole('textbox', { name: 'New branch name:' }).fill(branch); + await this.branchesDialog.getByRole('button', { name: 'Create', exact: true }).click(); await expect(this.branchesDialog.getByText(`${branch} *`)).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); @@ -478,8 +529,8 @@ export class GitSyncPage extends BasePage { */ async checkoutBranch(branch: string): Promise { await this.branchesDialog - .getByRole("row", { name: branch, exact: true }) - .getByRole("button", { name: "Checkout" }) + .getByRole('row', { name: branch, exact: true }) + .getByRole('button', { name: 'Checkout' }) .click(); } @@ -489,12 +540,12 @@ export class GitSyncPage extends BasePage { * @param branch - The branch to delete */ async deleteBranch(branch: string): Promise { - const row = this.branchesDialog.getByRole("row", { + const row = this.branchesDialog.getByRole('row', { name: branch, exact: true, }); - await row.getByRole("button", { name: "Delete" }).click(); - await row.getByRole("button", { name: "Confirm" }).click(); + await row.getByRole('button', { name: 'Delete' }).click(); + await row.getByRole('button', { name: 'Confirm' }).click(); } /** @@ -503,11 +554,8 @@ export class GitSyncPage extends BasePage { * @param branch - The branch to merge in */ async mergeBranch(branch: string): Promise { - await this.branchesDialog - .getByLabel(branch, { exact: true }) - .getByRole("button", { name: "Merge" }) - .click(); - await this.page.getByRole("button", { name: "Confirm" }).click(); + await this.branchesDialog.getByLabel(branch, { exact: true }).getByRole('button', { name: 'Merge' }).click(); + await this.page.getByRole('button', { name: 'Confirm' }).click(); } /** @@ -518,12 +566,8 @@ export class GitSyncPage extends BasePage { * @param branch - The branch name to attempt to create */ async submitNewBranch(branch: string): Promise { - await this.branchesDialog - .getByRole("textbox", { name: "New branch name:" }) - .fill(branch); - await this.branchesDialog - .getByRole("button", { name: "Create", exact: true }) - .click(); + await this.branchesDialog.getByRole('textbox', { name: 'New branch name:' }).fill(branch); + await this.branchesDialog.getByRole('button', { name: 'Create', exact: true }).click(); } /** @@ -533,7 +577,7 @@ export class GitSyncPage extends BasePage { * @returns The error text */ async getNewBranchError(): Promise { - const error = this.branchesDialog.locator("form p"); + const error = this.branchesDialog.locator('form p'); await expect(error).toBeVisible({ timeout: DEFAULT_TIMEOUT }); return (await error.innerText()).trim(); } @@ -545,7 +589,7 @@ export class GitSyncPage extends BasePage { * @returns The local branch names */ async getLocalBranches(): Promise { - return this.readBranchRows("Branches list"); + return this.readBranchRows('Branches list'); } /** @@ -555,10 +599,8 @@ export class GitSyncPage extends BasePage { * @returns The remote-only branch names */ async getRemoteBranches(): Promise { - await expect( - this.branchesDialog.getByText("Fetching remote branches..."), - ).toBeHidden({ timeout: DEFAULT_TIMEOUT }); - return this.readBranchRows("Remote Branches list"); + await expect(this.branchesDialog.getByText('Fetching remote branches...')).toBeHidden({ timeout: DEFAULT_TIMEOUT }); + return this.readBranchRows('Remote Branches list'); } /** @@ -569,10 +611,10 @@ export class GitSyncPage extends BasePage { */ async checkoutRemoteBranch(branch: string): Promise { await this.branchesDialog - .getByRole("grid", { name: "Remote Branches list", exact: true }) - .getByRole("row") + .getByRole('grid', { name: 'Remote Branches list', exact: true }) + .getByRole('row') .filter({ hasText: branch }) - .getByRole("button", { name: "Fetch and checkout" }) + .getByRole('button', { name: 'Fetch and checkout' }) .click(); await expect(this.branchesDialog.getByText(`${branch} *`)).toBeVisible({ timeout: DEFAULT_TIMEOUT, @@ -586,26 +628,22 @@ export class GitSyncPage extends BasePage { * close. Call after `mergeBranch()` or `pull()` hit conflicts. * @param version - "Current" keeps the local change, "Incoming" takes the merged-in one */ - async resolveMergeConflicts(version: "Current" | "Incoming"): Promise { + async resolveMergeConflicts(version: 'Current' | 'Incoming'): Promise { await expect(this.mergeConflictsDialog).toBeVisible({ timeout: DEFAULT_TIMEOUT, }); - const rows = this.mergeConflictsDialog - .getByRole("grid", { name: "Conflicted changes" }) - .getByRole("row"); + const rows = this.mergeConflictsDialog.getByRole('grid', { name: 'Conflicted changes' }).getByRole('row'); const count = await rows.count(); for (let i = 0; i < count; i++) { await rows.nth(i).click(); await this.mergeConflictsDialog - .getByRole("button", { + .getByRole('button', { name: `Take all ${version.toLowerCase()} changes`, }) .click(); - await this.page.getByRole("button", { name: "Confirm" }).click(); + await this.page.getByRole('button', { name: 'Confirm' }).click(); } - await this.mergeConflictsDialog - .getByRole("button", { name: "Resolve conflicts" }) - .click(); + await this.mergeConflictsDialog.getByRole('button', { name: 'Resolve conflicts' }).click(); await expect(this.mergeConflictsDialog).toBeHidden({ timeout: DEFAULT_TIMEOUT, }); @@ -615,30 +653,27 @@ export class GitSyncPage extends BasePage { * Closes the Branches modal. */ async closeBranchesDialog(): Promise { - await this.page.getByTestId("close-git-project-branches-modal").click(); + await this.page.getByTestId('close-git-project-branches-modal').click(); await this.page - .getByTestId("git-project-branches-modal-overlay") - .waitFor({ state: "hidden", timeout: DEFAULT_TIMEOUT }); + .getByTestId('git-project-branches-modal-overlay') + .waitFor({ state: 'hidden', timeout: DEFAULT_TIMEOUT }); } - private async readChangeRows( - list: "Staged changes" | "Unstaged changes", - ): Promise { - const rows = this.commitDialog - .getByRole("grid", { name: list, exact: true }) - .locator('[role="row"][data-key]'); + private async readChangeRows(list: GitChangeList): Promise { + const rows = this.commitDialog.getByRole('grid', { name: list, exact: true }).locator('[role="row"][data-key]'); const texts = await rows.allInnerTexts(); - return texts.map((text) => text.split("\n")[0].trim()); + return texts.map(text => text.split('\n')[0].trim()); } - private async readBranchRows( - list: "Branches list" | "Remote Branches list", - ): Promise { - const rows = this.branchesDialog - .getByRole("grid", { name: list, exact: true }) - .locator('[role="row"][data-key]'); + private async readBranchRows(list: 'Branches list' | 'Remote Branches list'): Promise { + const rows = this.branchesDialog.getByRole('grid', { name: list, exact: true }).locator('[role="row"][data-key]'); const texts = await rows.allInnerTexts(); - return texts.map((text) => text.split("\n")[0].replace(/\s*\*$/, "").trim()); + return texts.map(text => + text + .split('\n')[0] + .replace(/\s*\*$/, '') + .trim(), + ); } private async openSyncMenu(): Promise { diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/commit-clears-change.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/commit-clears-change.spec.ts new file mode 100644 index 0000000000..b7df737c94 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/commit-clears-change.spec.ts @@ -0,0 +1,44 @@ +import { faker } from "@faker-js/faker"; + +import { ProjectType } from "../../../enums/project-types"; +import { + DEFAULT_TIMEOUT, + expect, + GIT_CREDENTIAL, + pushServerFile, + test, +} from "../../../misc/git-fixtures"; +import { Project } from "../../../models/project"; +import { buildReorderedCollectionFile } from "./helpers"; + +test("Verify committing a hand-reordered file once leaves the working tree clean", async ({ + user, +}) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ + path: fileName, + content: buildReorderedCollectionFile(faker.string.alpha(10)).content, + }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create( + new Project(faker.string.alphanumeric(10), ProjectType.Git), + GIT_CREDENTIAL.name, + ); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + const message = faker.string.alphanumeric(10); + const commit = await gitSyncFlow.commit(message); + + await expect + .poll(async () => gitSyncPage.hasUncommittedChanges(), { + timeout: DEFAULT_TIMEOUT, + }) + .toBe(false); + expect(commit.message).toBe(message); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/confirming-discard-brings-change-back.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/confirming-discard-brings-change-back.spec.ts new file mode 100644 index 0000000000..c2150d2ff0 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/confirming-discard-brings-change-back.spec.ts @@ -0,0 +1,35 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Project } from '../../../models/project'; +import { buildReorderedCollectionFile } from './helpers'; + +test('Verify confirming discard on a hand-reordered file brings the same change straight back', async ({ user }) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ + path: fileName, + content: buildReorderedCollectionFile(faker.string.alpha(10)).content, + }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create(new Project(faker.string.alphanumeric(10), ProjectType.Git), GIT_CREDENTIAL.name); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + await gitSyncFlow.discardChangeAfterReview(fileName); + + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + expect(await gitSyncPage.isUndiscardableWarningVisible()).toBe(true); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/helpers.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/helpers.ts new file mode 100644 index 0000000000..6a88be456c --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/helpers.ts @@ -0,0 +1,58 @@ +import { faker } from "@faker-js/faker"; + +export const UNDISCARDABLE_WARNING_TEXT = + "This change is only Insomnia internally reformatting the file"; + +export function buildReorderedCollectionFile( + collectionName: string, + options: { oldSchema?: boolean } = {}, +) { + const id = faker.string.alphanumeric({ length: 16, casing: "lower" }); + const folders = [ + { id: `fld_b${id}`, name: faker.string.alpha(10), created: 3000 }, + { id: `fld_a${id}`, name: faker.string.alpha(10), created: 2000 }, + ]; + const content = [ + "type: collection.insomnia.rest/5.0", + ...(options.oldSchema ? [] : ['schema_version: "5.1"']), + `name: ${collectionName}`, + "meta:", + ` id: wrk_${id}`, + " created: 1000", + " modified: 1000", + "collection:", + ...folders.flatMap((folder) => [ + ` - name: ${folder.name}`, + " meta:", + ` id: ${folder.id}`, + ` created: ${folder.created}`, + ` modified: ${folder.created}`, + " children:", + ` - url: https://example.com/${folder.id}`, + ` name: Request in ${folder.name}`, + " meta:", + ` id: req_${folder.id}`, + " created: 5000", + " modified: 5000", + " method: GET", + " headers:", + " - name: X-Folder", + ` value: ${folder.id}`, + ...(options.oldSchema ? [" id: pair_1"] : []), + ]), + "cookieJar:", + " name: Default Jar", + " meta:", + ` id: jar_${id}`, + " created: 1000", + " modified: 1000", + "environments:", + " name: Base Environment", + " meta:", + ` id: env_${id}`, + " created: 1000", + " modified: 1000", + "", + ].join("\n"); + return { content, requestName: `Request in ${folders[0].name}` }; +} diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/mixed-change-is-not-flagged.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/mixed-change-is-not-flagged.spec.ts new file mode 100644 index 0000000000..fce9538cb1 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/mixed-change-is-not-flagged.spec.ts @@ -0,0 +1,47 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Collection } from '../../../models/collection'; +import { Project } from '../../../models/project'; +import { buildReorderedCollectionFile } from './helpers'; + +test('Verify a real edit mixed with a reorder is not flagged until the real edit is discarded', async ({ user }) => { + const { gitSyncFlow, httpRequestFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + const collectionName = faker.string.alpha(10); + const { content, requestName } = buildReorderedCollectionFile(collectionName); + await pushServerFile({ path: fileName, content }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create(new Project(faker.string.alphanumeric(10), ProjectType.Git), GIT_CREDENTIAL.name); + const collection = new Collection(collectionName); + const request = await httpRequestFlow.get(requestName, collection); + await workspaceFlow.rename(request!, faker.string.alphanumeric(10)); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + const warningWithMixedChange = await gitSyncPage.isUndiscardableWarningVisible(); + await gitSyncPage.closeCommitDialog(); + await gitSyncFlow.discardChangeAfterReview(fileName); + + expect(warningWithMixedChange).toBe(false); + await expect + .poll( + async () => { + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + const hasWarning = await gitSyncPage.isUndiscardableWarningVisible(); + await gitSyncPage.closeCommitDialog(); + return hasWarning; + }, + { timeout: DEFAULT_TIMEOUT }, + ) + .toBe(true); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/non-insomnia-added-and-deleted-files-show-no-warning.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/non-insomnia-added-and-deleted-files-show-no-warning.spec.ts new file mode 100644 index 0000000000..d2c5d13645 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/non-insomnia-added-and-deleted-files-show-no-warning.spec.ts @@ -0,0 +1,62 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Collection } from '../../../models/collection'; +import { Project } from '../../../models/project'; + +test('Verify a modified non-Insomnia file and an added or deleted file show no undiscardable warning', async ({ + user, +}) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const otherFileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ path: otherFileName, content: `openapi: 3.0.0\ninfo:\n title: ${faker.string.alpha(10)}\n` }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + const cloneParentDir = fs.mkdtempSync(path.join(os.tmpdir(), 'insomnia-undiscardable-')); + const project = await workspaceFlow.create( + new Project(faker.string.alphanumeric(10), ProjectType.Git), + GIT_CREDENTIAL.name, + { cloneParentDir }, + ); + const deletedFile = faker.string.alpha(10).toLowerCase(); + const deletedCollection = await workspaceFlow.create( + project, + new Collection(faker.string.alphanumeric(10)), + deletedFile, + ); + await gitSyncFlow.commit(faker.string.alphanumeric(10)); + const cloneDir = path.join(cloneParentDir, fs.readdirSync(cloneParentDir)[0]); + fs.appendFileSync(path.join(cloneDir, otherFileName), ` version: ${faker.string.numeric(3)}\n`); + const addedFile = faker.string.alpha(11).toLowerCase(); + await workspaceFlow.create(project, new Collection(faker.string.alphanumeric(10)), addedFile); + await workspaceFlow.delete(deletedCollection); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged.length, { + timeout: DEFAULT_TIMEOUT, + }) + .toBeGreaterThanOrEqual(3); + const unstaged = (await gitSyncFlow.getChanges()).unstaged; + const warnings = []; + for (const file of unstaged) { + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, file); + warnings.push(await gitSyncPage.isUndiscardableWarningVisible()); + await gitSyncPage.closeCommitDialog(); + } + + expect(unstaged).toEqual( + expect.arrayContaining([ + expect.stringContaining(otherFileName), + expect.stringContaining(addedFile), + expect.stringContaining(deletedFile), + ]), + ); + expect(warnings).toEqual(unstaged.map(() => false)); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/old-schema-file-flagged.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/old-schema-file-flagged.spec.ts new file mode 100644 index 0000000000..a9827ae17f --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/old-schema-file-flagged.spec.ts @@ -0,0 +1,34 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Project } from '../../../models/project'; +import { buildReorderedCollectionFile } from './helpers'; + +test('Verify a file from an older schema that Insomnia only normalizes is flagged as undiscardable', async ({ + user, +}) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ + path: fileName, + content: buildReorderedCollectionFile(faker.string.alpha(10), { + oldSchema: true, + }).content, + }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create(new Project(faker.string.alphanumeric(10), ProjectType.Git), GIT_CREDENTIAL.name); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + + expect(await gitSyncPage.isUndiscardableWarningVisible()).toBe(true); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/real-edit-shows-no-warning.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/real-edit-shows-no-warning.spec.ts new file mode 100644 index 0000000000..52382c1371 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/real-edit-shows-no-warning.spec.ts @@ -0,0 +1,42 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { HttpMethod } from '../../../enums/http-method'; +import { ProjectType } from '../../../enums/project-types'; +import { HTTP_SERVER } from '../../../misc/constants'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, test } from '../../../misc/git-fixtures'; +import { Collection } from '../../../models/collection'; +import { Project } from '../../../models/project'; + +test('Verify a real edit shows no undiscardable warning in the diff or the discard confirmation', async ({ user }) => { + const { gitSyncFlow, httpRequestFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + const project = await workspaceFlow.create( + new Project(faker.string.alphanumeric(10), ProjectType.Git), + GIT_CREDENTIAL.name, + ); + const fileName = faker.string.alpha(10).toLowerCase(); + const collection = await workspaceFlow.create(project, new Collection(faker.string.alphanumeric(10)), fileName); + const request = await httpRequestFlow.create(collection, { + name: faker.string.alphanumeric(10), + method: HttpMethod.Get, + url: `${HTTP_SERVER}/cookies`, + }); + await gitSyncFlow.commit(faker.string.alphanumeric(10)); + await workspaceFlow.rename(request, faker.string.alphanumeric(10)); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged.length, { + timeout: DEFAULT_TIMEOUT, + }) + .toBeGreaterThan(0); + const unstaged = (await gitSyncFlow.getChanges()).unstaged[0]; + + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, unstaged); + expect(await gitSyncPage.isUndiscardableWarningVisible()).toBe(false); + + await gitSyncPage.openDiscardConfirmation(unstaged); + expect(await gitSyncPage.isUndiscardableWarningVisibleInDiscardDialog()).toBe(false); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/staged-file-shows-no-warning.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/staged-file-shows-no-warning.spec.ts new file mode 100644 index 0000000000..b70144b76e --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/staged-file-shows-no-warning.spec.ts @@ -0,0 +1,30 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Project } from '../../../models/project'; +import { buildReorderedCollectionFile } from './helpers'; + +test('Verify a staged hand-reordered file shows no undiscardable warning', async ({ user }) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ + path: fileName, + content: buildReorderedCollectionFile(faker.string.alpha(10)).content, + }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create(new Project(faker.string.alphanumeric(10), ProjectType.Git), GIT_CREDENTIAL.name); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + await gitSyncFlow.stageChanges([fileName]); + + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Staged, fileName); + expect(await gitSyncPage.isUndiscardableWarningVisible()).toBe(false); +}); diff --git a/packages/insomnia-test/tests/git-sync/undiscardable-change/warning-in-diff-and-discard-confirmation.spec.ts b/packages/insomnia-test/tests/git-sync/undiscardable-change/warning-in-diff-and-discard-confirmation.spec.ts new file mode 100644 index 0000000000..5ce1807780 --- /dev/null +++ b/packages/insomnia-test/tests/git-sync/undiscardable-change/warning-in-diff-and-discard-confirmation.spec.ts @@ -0,0 +1,36 @@ +import { faker } from '@faker-js/faker'; + +import { GitChangeList } from '../../../enums/git-change-list'; +import { ProjectType } from '../../../enums/project-types'; +import { DEFAULT_TIMEOUT, expect, GIT_CREDENTIAL, pushServerFile, test } from '../../../misc/git-fixtures'; +import { Project } from '../../../models/project'; +import { buildReorderedCollectionFile, UNDISCARDABLE_WARNING_TEXT } from './helpers'; + +test('Verify a hand-reordered file warns that the change cannot be discarded in both the diff and the discard confirmation', async ({ + user, +}) => { + const { gitSyncFlow, preferencesFlow, workspaceFlow } = user.flowManager; + const { gitSyncPage } = user.pageManager; + + const fileName = `${faker.string.alpha(10).toLowerCase()}.yaml`; + await pushServerFile({ + path: fileName, + content: buildReorderedCollectionFile(faker.string.alpha(10)).content, + }); + await preferencesFlow.addGitCredential(GIT_CREDENTIAL); + await workspaceFlow.create(new Project(faker.string.alphanumeric(10), ProjectType.Git), GIT_CREDENTIAL.name); + await expect + .poll(async () => (await gitSyncFlow.getChanges()).unstaged, { + timeout: DEFAULT_TIMEOUT, + }) + .toContain(fileName); + + await gitSyncPage.openCommitDialog(); + await gitSyncPage.openChangeDiff(GitChangeList.Unstaged, fileName); + const hasDiffWarning = await gitSyncPage.isUndiscardableWarningVisible(); + await gitSyncPage.openDiscardConfirmation(fileName); + + expect(hasDiffWarning).toBe(true); + expect(await gitSyncPage.isUndiscardableWarningVisibleInDiscardDialog()).toBe(true); + expect(await gitSyncPage.getDiscardConfirmationText()).toContain(UNDISCARDABLE_WARNING_TEXT); +});