Skip to content
Merged
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
169 changes: 169 additions & 0 deletions .github/workflows/ui-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
name: UI Tests

on:
push:
branches:
- develop
pull_request:
workflow_dispatch:

concurrency:
group: ui-tests-${{ github.event.number || github.ref }}
cancel-in-progress: true
Comment thread
coderabbitai[bot] marked this conversation as resolved.

jobs:
ui-tests:
runs-on: ubuntu-latest
timeout-minutes: 60
name: Playwright E2E

services:
redis-cache:
image: redis:alpine
ports:
- 13000:6379
redis-queue:
image: redis:alpine
ports:
- 11000:6379
mariadb:
image: mariadb:10.6
env:
MYSQL_ROOT_PASSWORD: root
ports:
- 3306:3306
options: --health-cmd="mariadb-admin ping" --health-interval=5s --health-timeout=2s --health-retries=3

steps:
- name: Clone
uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: "3.14"

- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: 24
check-latest: true

- name: Add to Hosts
run: echo "127.0.0.1 forms.test" | sudo tee -a /etc/hosts

- name: Cache pip
uses: actions/cache@v5
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml', '**/setup.py', '**/setup.cfg') }}
restore-keys: |
${{ runner.os }}-pip-
${{ runner.os }}-

- name: Get yarn cache directory path
id: yarn-cache-dir-path
run: 'echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT'

- uses: actions/cache@v5
id: yarn-cache
with:
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ runner.os }}-yarn-

- name: Cache Playwright browsers
uses: actions/cache@v5
with:
path: ~/.cache/ms-playwright
key: ${{ runner.os }}-playwright-${{ hashFiles('**/package.json') }}
restore-keys: |
${{ runner.os }}-playwright-

- name: Install MariaDB Client
run: |
sudo apt update
sudo apt-get install mariadb-client

- name: Setup Frappe bench
run: |
pip install frappe-bench
bench init --skip-redis-config-generation --skip-assets --python "$(which python)" ~/frappe-bench
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL character_set_server = 'utf8mb4'"
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"

- name: Install app
working-directory: /home/runner/frappe-bench
run: |
bench get-app frappe_factory_bot https://github.com/harshtandiya/frappe_factory_bot.git --branch main
bench get-app forms_pro $GITHUB_WORKSPACE
bench setup requirements --dev
bench new-site --db-root-password root --admin-password admin forms.test
bench --site forms.test install-app forms_pro
bench build
env:
CI: "Yes"

- name: Configure Site for UI Tests
working-directory: /home/runner/frappe-bench
run: |
bench --site forms.test set-config allow_tests true
bench --site forms.test set-config server_script_enabled true
bench --site forms.test set-config host_name "http://forms.test:8000"
bench --site forms.test set-config ignore_csrf 1
bench --site forms.test set-config mute_emails 1

- name: Setup E2E test data
working-directory: /home/runner/frappe-bench
run: bench --site forms.test execute forms_pro.install.before_tests

- name: Start Frappe Server
working-directory: /home/runner/frappe-bench
run: |
sed -i 's/^watch:/# watch:/g' Procfile
sed -i 's/^schedule:/# schedule:/g' Procfile
bench start &> /tmp/bench_start.log &
echo "Waiting for Frappe server to start..."
timeout 90 bash -c 'until curl -s http://forms.test:8000 > /dev/null; do sleep 2; done' || (echo "=== bench start log ===" && cat /tmp/bench_start.log && exit 1)
echo "Frappe server is ready!"

- name: Install Playwright browsers
working-directory: ${{ github.workspace }}/frontend
run: |
yarn install
npx playwright install --with-deps chromium

- name: Run Playwright tests
working-directory: ${{ github.workspace }}/frontend
run: yarn test:e2e
env:
BASE_URL: http://forms.test:8000
TEST_USER_EMAIL: test_forms_pro_user@example.com
TEST_USER_PASSWORD: testforms123
CI: "true"

- name: Upload HTML report
if: always()
uses: actions/upload-artifact@v4
with:
name: playwright-report
path: frontend/e2e/playwright-report/
retention-days: 14

- name: Upload failure artifacts
if: failure()
uses: actions/upload-artifact@v4
with:
name: playwright-results
path: frontend/e2e/test-results/
retention-days: 7

- name: Show bench logs on failure
if: failure()
run: |
echo "=== bench start log ==="
cat /tmp/bench_start.log || true
echo ""
echo "=== Frappe logs ==="
cat /home/runner/frappe-bench/logs/*.log || true
7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,9 @@ jspm_packages/
/forms_pro/www/frontend.html
/forms_pro/public/node_modules
frontend/components.d.ts
frontend/auto-imports.d.ts
frontend/auto-imports.d.ts

# Playwright
frontend/e2e/auth/storageState.json
frontend/e2e/test-results/
frontend/e2e/playwright-report/
18 changes: 18 additions & 0 deletions forms_pro/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ def create_user_forms_module():
def before_tests():
give_admin_forms_pro_role()
create_test_user()
_ensure_admin_has_default_team()


def _ensure_admin_has_default_team():
from forms_pro.overrides.roles import create_default_team_for_user
from forms_pro.utils.teams import get_user_teams

if get_user_teams("Administrator"):
return
Comment on lines +29 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid side-effecting helper calls for existence checks in install hooks.

Line 32 uses get_user_teams("Administrator") as a predicate, but that helper can mutate defaults. For setup hooks, prefer a read-only existence check.

Suggested change
 def _ensure_admin_has_default_team():
     from forms_pro.overrides.roles import create_default_team_for_user
-    from forms_pro.utils.teams import get_user_teams

-    if get_user_teams("Administrator"):
+    if frappe.db.exists("FP Team Member", {"user": "Administrator"}):
         return
     admin = frappe.get_doc("User", "Administrator")
     create_default_team_for_user(admin)
📝 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
from forms_pro.overrides.roles import create_default_team_for_user
from forms_pro.utils.teams import get_user_teams
if get_user_teams("Administrator"):
return
from forms_pro.overrides.roles import create_default_team_for_user
if frappe.db.exists("FP Team Member", {"user": "Administrator"}):
return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@forms_pro/install.py` around lines 29 - 33, get_user_teams is a helper that
can create defaults and must not be called for a simple existence check in the
install hook; replace the predicate usage of get_user_teams("Administrator")
with a read-only existence check (e.g., a new helper like
team_role_exists("Administrator") or a direct non-mutating DB query against the
Team/Role model) so the install hook doesn't trigger side effects, update
imports accordingly, and leave create_default_team_for_user for actual creation
logic only.

admin = frappe.get_doc("User", "Administrator")
create_default_team_for_user(admin)


def give_admin_forms_pro_role():
Expand All @@ -31,6 +42,8 @@ def give_admin_forms_pro_role():


def create_test_user():
from frappe.utils.password import update_password

if frappe.db.exists("User", FORMS_PRO_TEST_USER):
return

Expand All @@ -39,5 +52,10 @@ def create_test_user():
user.first_name = "Test"
user.last_name = "Forms Pro User"
user.insert(ignore_permissions=True)

# Frappe auto-assigns System User on insert; replace with only Forms Pro User
user.roles = []
user.append("roles", {"role": FORMS_PRO_ROLE})
user.save(ignore_permissions=True)

update_password(FORMS_PRO_TEST_USER, "testforms123")
2 changes: 1 addition & 1 deletion forms_pro/utils/form_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def generate(self) -> None:
self._initialize_doctype()
self._add_status_field()
self._initialize_form_document()
frappe.clear_cache()
frappe.clear_document_cache("DocType", self.doctype.name)

def _initialize_doctype(self) -> None:
if self.doctype:
Expand Down
98 changes: 98 additions & 0 deletions frontend/e2e/fixtures/test-data.fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {
test as base,
request,
type APIRequestContext,
} from "@playwright/test";

type TestDataFixtures = {
apiContext: APIRequestContext;
getTeamId: () => Promise<string>;
createForm: () => Promise<string>;
createPublishedForm: () => Promise<{ formId: string; route: string }>;
submitForm: (formId: string) => Promise<void>;
};

async function fetchTeamId(apiContext: APIRequestContext): Promise<string> {
const res = await apiContext.get(
"/api/method/forms_pro.api.user.get_user_teams"
);
const { message } = await res.json();
if (!message?.length) throw new Error("No team found for test user");
return message[0].name as string;
}

export const test = base.extend<TestDataFixtures>({
apiContext: async ({}, use) => {
const ctx = await request.newContext({
baseURL: process.env.BASE_URL ?? "http://localhost:8001",
storageState: "./e2e/auth/storageState.json",
});
await use(ctx);
await ctx.dispose();
},

getTeamId: async ({ apiContext }, use) => {
await use(() => fetchTeamId(apiContext));
},

createForm: async ({ apiContext }, use) => {
const created: string[] = [];

await use(async () => {
const teamId = await fetchTeamId(apiContext);
const res = await apiContext.post(
"/api/method/forms_pro.utils.form_generator.create_form",
{ data: { team_id: teamId } }
);
const { message } = await res.json();
const formId = message.form_document as string;
created.push(formId);
return formId;
});

// Teardown: delete all created forms
for (const id of created) {
await apiContext.delete(`/api/resource/Form/${id}`).catch(() => {});
}
},

createPublishedForm: async ({ apiContext }, use) => {
const created: string[] = [];

await use(async () => {
const teamId = await fetchTeamId(apiContext);
const createRes = await apiContext.post(
"/api/method/forms_pro.utils.form_generator.create_form",
{ data: { team_id: teamId } }
);
const { message } = await createRes.json();
const formId = message.form_document as string;
created.push(formId);

// Publish the form via Frappe REST API
const publishRes = await apiContext.put(`/api/resource/Form/${formId}`, {
data: { is_published: 1 },
});
const publishData = await publishRes.json();
const route = publishData.data?.route as string;

return { formId, route };
});

for (const id of created) {
await apiContext.delete(`/api/resource/Form/${id}`).catch(() => {});
}
},

// Creates a guest submission against an already-published form
submitForm: async ({ apiContext }, use) => {
await use(async (formId: string) => {
await apiContext.post(
"/api/method/forms_pro.api.submission.submit_form_response",
{ data: { form_id: formId, form_data: [] } }
);
});
},
});

export { expect } from "@playwright/test";
28 changes: 28 additions & 0 deletions frontend/e2e/global-setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { chromium, type FullConfig } from "@playwright/test";
import * as fs from "fs";
import * as path from "path";
import { fileURLToPath } from "url";

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const AUTH_FILE = path.join(__dirname, "auth/storageState.json");

export default async function globalSetup(_config: FullConfig) {
fs.mkdirSync(path.dirname(AUTH_FILE), { recursive: true });

const browser = await chromium.launch();
const context = await browser.newContext();
const page = await context.newPage();

const baseURL = process.env.BASE_URL ?? "http://localhost:8001";

await page.goto(baseURL);
await page.request.post(`${baseURL}/api/method/login`, {
form: {
usr: process.env.TEST_USER_EMAIL ?? "test_forms_pro_user@example.com",
pwd: process.env.TEST_USER_PASSWORD ?? "testforms123",
},
});

await context.storageState({ path: AUTH_FILE });
await browser.close();
Comment on lines +19 to +27
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fail fast if login request fails before writing storage state.

Right now, Line 19-Line 27 can persist an unauthenticated state when login fails, which turns into opaque downstream test failures.

🛠️ Suggested fix
-  await page.request.post(`${baseURL}/api/method/login`, {
+  const loginRes = await page.request.post(`${baseURL}/api/method/login`, {
     form: {
       usr: "Administrator",
       pwd: process.env.TEST_ADMIN_PASSWORD ?? "admin",
     },
   });
+  if (!loginRes.ok()) {
+    throw new Error(
+      `Global setup login failed: ${loginRes.status()} ${loginRes.statusText()}`
+    );
+  }
 
   await context.storageState({ path: AUTH_FILE });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/e2e/global-setup.ts` around lines 19 - 27, Ensure the login POST via
page.request.post is validated before persisting session: check the response
(e.g., response.ok or response.status === 200 and any expected body fields) from
page.request.post and throw or exit on failure so context.storageState({ path:
AUTH_FILE }) and browser.close() only run for a successful login; include
failure context (status/text) in the thrown error or log to make the cause
clear.

}
14 changes: 14 additions & 0 deletions frontend/e2e/helpers/dashboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import type { Page } from "@playwright/test";

export class DashboardPage {
constructor(private page: Page) {}

async goto() {
await this.page.goto("/forms");
}

// Form title headings inside FormPreviewCard
formTitles() {
return this.page.getByRole("heading", { level: 3 });
}
}
Loading
Loading