github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-04 17:10:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Follow the branching UI decisions in the Playwright specs

Browse Source 
Twelve specs of branches-library, merged-and-archived-branches, and
pull-requests failed on iop-vc-preview before penpot#12780. They still
asserted copy and flows that the branching UI work replaced on purpose:

- The banner of a review now says "Reviewing the design review", and
  its cancel confirmation accepts with "Cancel design review".
- The compare dialog is titled "Review changes".
- A library's header says "Default branch". The popover adds the note
  "Read by library consumers" under the main row.
- Restoring an archived branch asks for confirmation first.
- A merge deletes its branch (penpot#12596), so neither the switcher
  nor the all-branches dialog lists merged branches.

The specs now assert the current behaviour, including that no merged
group or tab shows.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Álvaro Tejero Cantero 2026-11-03 12:49:29 +01:00
parent d107c39752
commit cd3ba19456
No known key found for this signature in database
3 changed files with 34 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
frontend/playwright/ui/specs/branches-library.spec.js
View File

@ -33,10 +33,9 @@ test("on a branch of a library, the header says consumers read main", async ({
);
await expect(chip).toBeVisible();
// the popover says it under the main row
await chip.click();
await expect(
page.getByText("Default branch (read by library consumers)"),
).toBeVisible();
await expect(page.getByText("Read by library consumers")).toBeVisible();
});
test("on a library, the create dialog says consumers read main", async ({
@ -51,11 +50,11 @@ test("on a library, the create dialog says consumers read main", async ({
await workspacePage.mockGetFile("pull-requests/get-file-blank-shared.json");
await workspacePage.goToWorkspace();
await expect(
page.getByText("Default branch (read by library consumers)"),
).toBeVisible();
await expect(page.getByText("Default branch", { exact: true })).toBeVisible();
// the header says only "Default branch": the popover adds the library note
await page.getByTitle("Switch branch").click();
await expect(page.getByText("Read by library consumers")).toBeVisible();
await page.getByText("New branch").click();
await expect(
page.getByText(

30
frontend/playwright/ui/specs/merged-and-archived-branches.spec.js
View File

@ -60,7 +60,8 @@ test("an archived branch opens read-only and offers its ways out", async ({
page.getByRole("toolbar", { name: "Drawing tools" }),
).toBeHidden();
// restoring reopens the branch, and the banner follows its new status
// restoring asks first, then reopens the branch, and the banner follows
// its new status
await workspacePage.mockRPC(
"get-file-branch-info?file-id=*",
"pull-requests/get-file-branch-info-branch.json",
@ -71,6 +72,13 @@ test("an archived branch opens read-only and offers its ways out", async ({
/"~:archived":\s*false/.test(req.postData() ?? ""),
);
await banner.getByRole("button", { name: "Restore" }).click();
// the confirmation is a plain modal: its title heads the box that holds
// the accept button
await page
.getByRole("heading", { name: "Restore branch" })
.locator("../..")
.getByRole("button", { name: "Restore" })
.click();
await restore;
await expect(banner.getByText("1 change not in main")).toBeVisible();
await expect(
@ -94,7 +102,7 @@ test("an archived branch asks before turning into an ordinary file", async ({
).toBeVisible();
});
test("the switcher groups merged and archived branches apart", async ({
test("the switcher groups archived branches apart and lists no merged ones", async ({
page,
}) => {
const workspacePage = new WorkspacePage(page);
@ -104,8 +112,9 @@ test("the switcher groups merged and archived branches apart", async ({
await page.getByTitle("Switch branch").click();
const popover = page.getByRole("dialog", { name: "Branches" });
await popover.getByRole("button", { name: /^Merged/ }).click();
await expect(popover.getByText("shipped")).toBeVisible();
// a merge deletes its branch: no group lists merged branches
await expect(popover.getByRole("button", { name: /^Merged/ })).toBeHidden();
await expect(popover.getByText("shipped")).toBeHidden();
await expect(popover.getByText("parked")).toBeHidden();
await popover.getByRole("button", { name: /^Archived/ }).click();
@ -123,17 +132,8 @@ test("the branch list offers Restore for archived branches only", async ({
await page.getByTitle("Switch branch").click();
await page.getByText("All branches").click();
await page.getByRole("tab", { name: "Merged (1)" }).click();
await page.getByRole("button", { name: "Options", exact: true }).click();
await expect(
page.getByRole("menuitem", { name: "Branch info" }),
).toBeVisible();
await expect(page.getByRole("menuitem", { name: "Restore" })).toBeHidden();
await expect(page.getByRole("menuitem", { name: "Compare" })).toBeHidden();
await expect(
page.getByRole("menuitem", { name: "Turn into an ordinary file" }),
).toBeHidden();
await page.keyboard.press("Escape");
// a merge deletes its branch: no tab lists merged branches
await expect(page.getByRole("tab", { name: /^Merged/ })).toBeHidden();
await page.getByRole("tab", { name: "Archived (1)" }).click();
await page.getByRole("button", { name: "Options", exact: true }).click();

25
frontend/playwright/ui/specs/pull-requests.spec.js
View File

@ -3,6 +3,9 @@ import { WorkspacePage } from "../pages/WorkspacePage";
const PR_ID = "c7ce0794-0992-8105-8004-38f280443999";
const reviewTitle = (page) =>
page.getByText("Reviewing the design review", { exact: true });
test.beforeEach(async ({ page }) => {
await WorkspacePage.init(page);
await WorkspacePage.mockConfigFlags(page, ["enable-branching"]);
@ -30,11 +33,11 @@ test("opening a file with pr-id shows the review sandbox banner", async ({
await setupPullRequestSandbox(workspacePage);
await workspacePage.goToWorkspace({ extraParams: `&pr-id=${PR_ID}` });
await expect(page.getByText("Reviewing pull request")).toBeVisible();
await expect(reviewTitle(page)).toBeVisible();
await expect(page.getByText("Checkout redesign").first()).toBeVisible();
// the route reads as two pills: the branch under review, then main
const banner = page.getByText("Reviewing pull request").locator("../../..");
const banner = reviewTitle(page).locator("../../..");
await expect(banner.getByText("checkout", { exact: true })).toBeVisible();
await expect(banner.getByText("Main", { exact: true })).toBeVisible();
@ -58,7 +61,7 @@ test("the compare dialog shows the proposed changes", async ({ page }) => {
await page.getByRole("button", { name: "Compare" }).click();
// the branching compare dialog is reused to inspect the changes
await expect(page.getByText("Compare changes")).toBeVisible();
await expect(page.getByText("Review changes")).toBeVisible();
});
test("accepting the pull request opens the merge dialog", async ({ page }) => {
@ -66,7 +69,7 @@ test("accepting the pull request opens the merge dialog", async ({ page }) => {
await setupPullRequestSandbox(workspacePage);
await workspacePage.goToWorkspace({ extraParams: `&pr-id=${PR_ID}` });
const banner = page.getByText("Reviewing pull request").locator("../../..");
const banner = reviewTitle(page).locator("../../..");
await banner.getByRole("button", { name: "Options" }).click();
await page.getByText("Merge to main").click();
@ -80,7 +83,7 @@ test("the review behaves like a normal, editable file", async ({ page }) => {
await setupPullRequestSandbox(workspacePage);
await workspacePage.goToWorkspace({ extraParams: `&pr-id=${PR_ID}` });
await expect(page.getByText("Reviewing pull request")).toBeVisible();
await expect(reviewTitle(page)).toBeVisible();
// the file is the live branch, opened normally: the design toolbar is
// available so reviewers can try and validate anything...
await expect(workspacePage.rectShapeButton).toBeVisible();
@ -115,7 +118,7 @@ test("closing the pull request asks for confirmation", async ({ page }) => {
);
await workspacePage.goToWorkspace({ extraParams: `&pr-id=${PR_ID}` });
const banner = page.getByText("Reviewing pull request").locator("../../..");
const banner = reviewTitle(page).locator("../../..");
await banner.getByRole("button", { name: "Options" }).click();
await page.getByText("Cancel review").click();
@ -124,7 +127,7 @@ test("closing the pull request asks for confirmation", async ({ page }) => {
// confirming closes the pull request; being inside its sandbox, the
// user is taken back to the file (no pr-id in the url)
await page.getByRole("button", { name: "Cancel pull request" }).click();
await page.getByRole("button", { name: "Cancel design review" }).click();
await expect(page).not.toHaveURL(/pr-id/);
});
@ -161,15 +164,15 @@ test("exiting the review takes the user back to the file", async ({ page }) => {
await setupPullRequestSandbox(workspacePage);
await workspacePage.goToWorkspace({ extraParams: `&pr-id=${PR_ID}` });
await expect(page.getByText("Reviewing pull request")).toBeVisible();
const banner = page.getByText("Reviewing pull request").locator("../../..");
await expect(reviewTitle(page)).toBeVisible();
const banner = reviewTitle(page).locator("../../..");
await banner.getByRole("button", { name: "Options" }).click();
await page.getByText("Exit review").click();
// exiting drops the review context and the pr-id param; the file (the
// live branch) stays as it was
await expect(page).not.toHaveURL(/pr-id/);
await expect(page.getByText("Reviewing pull request")).toBeHidden();
await expect(reviewTitle(page)).toBeHidden();
});
test("during a review the header names the branch under review", async ({
@ -186,7 +189,7 @@ test("during a review the header names the branch under review", async ({
// the review banner replaces the branch banner, so the header loads the
// branch context itself
await expect(page.getByText("Reviewing pull request")).toBeVisible();
await expect(reviewTitle(page)).toBeVisible();
await expect(page.getByTitle("On branch checkout from main")).toBeVisible();
});