From 81d89bf81d86fafc1f31fcf41dc9207def778b6a Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Fri, 28 Aug 2026 01:41:16 +0530 Subject: [PATCH 1/3] test: build booking fixtures with frappe_factory_bot factories Fixture setup in the backend suite is duplicated everywhere, and an informal factory layer has already grown by accident: test_buzz_team.py is imported by ten other test files for create_user / create_owned_team, and payload_for is a per-doctype default_attributes table in all but name. Adopt frappe_factory_bot properly, on one module first. buzz/api/booking/ test_booking.py is the tracer bullet: its setUpClass builds the whole association chain, so converting it exercises every factory added here. - Add buzz/tests/factories/ with factories for User, Buzz Team, Event Category, Event Host, Buzz Event, Event Ticket Type and Ticket Add-on. - Install frappe_factory_bot in CI via bench get-app. No install-app: the app ships no DocTypes, only a Python import. Deliberately not in required_apps, since production does not need a test library. - Add the writing-tests skill documenting the conventions and the traps below. Three Frappe-level traps the factories have to work around, all found the hard way and written into the skill: - A team inserted plainly as Administrator becomes Administrator's default team for every later run on that site, because create_default_team_for takes the first enabled Owner membership and process_booking commits, so the row survives rollback. setup_test_records() then fails with "Venue Test Venue belongs to another team." Hence BuzzTeamFactory.create_owned_by(). - User.throttle_user_creation throws past 60 new users an hour. Test users are not rolled back, so minting a fresh one per fixture trips it after a couple of runs. Hence UserFactory.create_once() for fixed identities. - Faker's unique only dedupes within a process. Prompt-autonamed rows are the primary key and outlive the run that made them, so Event Category and Event Host suffix with frappe.generate_hash instead. No change to frappe_factory_bot was needed: the flags passthrough already carries both owner_user and ignore_permissions. Event Booking, Offline Payment Method and Buzz Coupon Code get no factory yet. Nothing here would exercise them; they land with the batch that does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016dbs2RcPoTwFTM2JTE386q --- .claude/skills/writing-tests/SKILL.md | 181 ++++++++++++++++++ .github/actions/setup-bench/action.yml | 4 + .gitignore | 1 + buzz/api/booking/test_booking.py | 149 +++----------- buzz/tests/__init__.py | 0 buzz/tests/factories/__init__.py | 17 ++ buzz/tests/factories/buzz_event_factory.py | 40 ++++ buzz/tests/factories/buzz_team_factory.py | 37 ++++ .../tests/factories/event_category_factory.py | 17 ++ buzz/tests/factories/event_host_factory.py | 21 ++ .../factories/event_ticket_type_factory.py | 29 +++ buzz/tests/factories/ticket_add_on_factory.py | 25 +++ buzz/tests/factories/user_factory.py | 36 ++++ 13 files changed, 439 insertions(+), 118 deletions(-) create mode 100644 .claude/skills/writing-tests/SKILL.md create mode 100644 buzz/tests/__init__.py create mode 100644 buzz/tests/factories/__init__.py create mode 100644 buzz/tests/factories/buzz_event_factory.py create mode 100644 buzz/tests/factories/buzz_team_factory.py create mode 100644 buzz/tests/factories/event_category_factory.py create mode 100644 buzz/tests/factories/event_host_factory.py create mode 100644 buzz/tests/factories/event_ticket_type_factory.py create mode 100644 buzz/tests/factories/ticket_add_on_factory.py create mode 100644 buzz/tests/factories/user_factory.py diff --git a/.claude/skills/writing-tests/SKILL.md b/.claude/skills/writing-tests/SKILL.md new file mode 100644 index 00000000..140a7e57 --- /dev/null +++ b/.claude/skills/writing-tests/SKILL.md @@ -0,0 +1,181 @@ +--- +name: writing-tests +description: How backend tests build their fixtures in Buzz — factories under buzz/tests/factories/ powered by frappe_factory_bot, instead of raw frappe.get_doc({...}).insert(). Covers authoring a factory, traits, overrides, the flags passthrough, and the Frappe-level traps (per-class rollback, prompt autoname, the User creation throttle, doc cache). Use this whenever writing or modifying a python test under buzz/, and when the user says "add a test", "write tests for X", "convert these tests", or "this test needs a fixture". +--- + +# Writing backend tests in Buzz + +Fixtures come from factories in `buzz/tests/factories/`, built on `frappe_factory_bot` +(`apps/frappe_factory_bot`, repo `harshtandiya/frappe_factory_bot`). It is a bench-level +dev app — installed by `bench get-app` in `.github/actions/setup-bench`, deliberately +**not** in `required_apps`, because production Buzz does not need it. + +Faker comes free: frappe depends on it, so `from faker import Faker` works. Do not add it +to `pyproject.toml`. + +## Rules + +1. **Never** `frappe.get_doc({...}).insert()` or `frappe.new_doc(...)` to build a fixture. + Use a factory. If the doctype has none, write it first. +2. One factory per doctype, `buzz/tests/factories/_factory.py`, class + `Factory(BaseFactory[])`. Parameterise the generic with the + real controller class so the IDE types the result. Re-export from `__init__.py`. +3. `default_attributes` sets only what `.insert()` actually needs. Fields with a DocType + default (`Event Ticket Type.currency` is `INR`, `Event Booking.status` is + `Approval Pending`) stay out. +4. **When the same override set turns up in a third test, promote it to a trait.** Overrides + are for one-offs; a trait is for a configuration that has a name — "a paid ticket type", + "a closed event", "a submitted booking". Promoting it puts those field values in one + place, so a doctype change is a one-line fix instead of a grep, and the call site starts + reading as the concept instead of as a bag of fields: + `EventTicketTypeFactory.create("paid", event=event)` says what the test needs, where + `create(price=500, event=event)` repeated six times makes the reader work it out every + time. Traits compose, so keep each to a single idea and apply several rather than + building one combined trait. Don't run ahead of the evidence, though — a configuration + used once is an override, and inventing traits before anything repeats just moves the + noise into the factory. +5. **Every default that hits a unique constraint must be unique per call.** Rollback is per + *class*, not per test, and three test files already comment on it + (`test_buzz_team.py:27`, `:155`, `test_buzz_event.py:165`) — a fixed default collides + with the previous test in the same class. `_fake.unique.*` covers that, but it only + dedupes within one process: for a value that is the doctype's **primary key** (anything + prompt-autonamed) use `frappe.generate_hash(length=8)`, because those rows outlive the + run that made them and Faker will eventually repeat itself. +6. Foreign keys honour the override before creating anything: + `self.overrides.get("event") or BuzzEventFactory.create().name`. Without this, passing + `event=` still spawns an orphan event. Import the related factory *inside* the + property, not at module top, or the imports cycle. +7. **Do not override `create()`.** It breaks `create_list` / `build_list`. A doctype needing + more than an insert gets a separate factory for the dependency, referenced from + `default_attributes` — or a small named classmethod alongside `create()` + (`BuzzTeamFactory.create_owned_by`, `UserFactory.create_once`). +8. No `__del_override__`. The base default is a no-op, cleanup is the per-class rollback, + and overriding it means the doc can be garbage-collected — and deleted — while a + downstream fixture still holds its name. +9. Child tables (`Event Booking Attendee`, `Additional Field`, …) get no factory. They go in + as nested dicts on the parent's attributes. + +## Authoring a factory + +Tabs, line length 110 — match `pyproject.toml`, not another app's style. + +```python +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.ticketing.doctype.event_ticket_type.event_ticket_type import EventTicketType + +_fake = Faker() + + +class EventTicketTypeFactory(BaseFactory[EventTicketType]): + doctype = "Event Ticket Type" + + @property + def default_attributes(self) -> dict[str, Any]: + from buzz.tests.factories.buzz_event_factory import BuzzEventFactory + + return { + "event": self.overrides.get("event") or BuzzEventFactory.create().name, + "title": f"Ticket {_fake.unique.word().capitalize()}", + "price": 0, + "is_published": 1, + } + + @property + def paid(self) -> dict[str, Any]: + return {"price": 500} +``` + +| Call | Returns | Saved? | +| --- | --- | --- | +| `Factory.build(*traits, **overrides)` | `T` | no | +| `Factory.create(*traits, **overrides)` | `T` | yes | +| `Factory.build_list(n, *traits, **overrides)` | `list[T]` | no | +| `Factory.create_list(n, *traits, **overrides)` | `list[T]` | yes | + +Precedence: overrides > traits > defaults. An unknown trait raises `TypeError`. + +## The `flags` passthrough + +`flags` is in Frappe's `RESERVED_KEYWORDS`, so `frappe.get_doc()` drops it from the +attribute dict. `BaseFactory.build` therefore applies it separately — which means **flags +only arrive through overrides, never through `default_attributes` or a trait**. + +```python +EventTicketTypeFactory.create(event=event, flags={"ignore_permissions": True}) +``` + +`insert()` leaves `flags.ignore_permissions` alone unless the kwarg is passed +(`frappe/model/document.py:709`), so the flag survives and `has_permission` short-circuits +on it. Use it where the old code passed `ignore_permissions=True`; a test running as +Administrator does not need it, and a test that is *about* permissions should not use it. + +The same route carries doctype flags the controller reads itself — +`Buzz Team.flags.owner_user` is how a team gets an Owner other than the session user. + +## Traps + +**Administrator must not own throwaway teams.** `create_default_team_for` picks the *first* +enabled Owner membership (`buzz_team.py:19`), and test rows are not always rolled back — +`process_booking` commits, so its fixtures survive. A team inserted plainly as Administrator +therefore becomes Administrator's "default team" for every later run on that site, and +`setup_test_records()` then fails with `Venue Test Venue belongs to another team.` Use +`BuzzTeamFactory.create_owned_by()`. + +**User creation is throttled.** `User.throttle_user_creation` throws `Throttled` past 60 new +users an hour (`throttle_user_limit`). Test users leak, so a suite that mints a fresh user +per fixture trips it after a couple of runs. Where the identity is fixed and one record is +all you want, use `UserFactory.create_once(email)`; use `create()` only when the test needs +a genuinely distinct user. + +**Prompt-autonamed doctypes need `name` in the attributes.** `Event Category` and +`Event Host` use `autoname: prompt`, and `_prompt_autoname` throws when `doc.name` is unset +(`frappe/model/naming.py:225`). Set `"name"` in `default_attributes`. + +**`before_insert` / `validate` can clobber an override.** Overrides are merged into the dict +that becomes the doc, and those hooks run after. Set the field after `.create()` and save +again. + +**The rollback restores a Single but not its cached copy** (`test_buzz_team_settings.py:47`). +A fixture touching `Buzz Team Settings` or `Buzz Settings` needs +`frappe.clear_document_cache`. + +## Consuming factories + +```python +import frappe +from frappe.tests import IntegrationTestCase + +from buzz.tests.factories import BuzzEventFactory, EventTicketTypeFactory, UserFactory + + +class BookingTestCase(IntegrationTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.booker = UserFactory.create_once("booking-owner@example.com").name + cls.event = BuzzEventFactory.create(flags={"ignore_permissions": True}) +``` + +`buzz/api/booking/test_booking.py` is the reference conversion. + +Some older test modules still export ad-hoc helpers — `create_user` / `create_owned_team` / +`payload_for` in `test_buzz_team.py`, `ensure_prompt_named_record` in `test_forms.py`, +`create_event` / `create_ticket` in `test_permissions.py`. They are being retired module by +module; `context_local.md` tracks who still imports what. Do not add new callers. + +## Running tests + +```bash +bench --site buzz.localhost run-tests --app buzz +bench --site buzz.localhost run-tests --module buzz.api.booking.test_booking +bench --site buzz.localhost run-tests --module buzz.api.booking.test_booking --test test_shape +``` + +Redis must be up (`bench start`, or `redis-server config/redis_cache.conf --daemonize yes` +and the same for `redis_queue.conf`) — without it global search sync asserts and every test +errors out. `testbuzz.localhost` is the CI-parity site and reproduces failures +`buzz.localhost` hides. diff --git a/.github/actions/setup-bench/action.yml b/.github/actions/setup-bench/action.yml index 43708d4b..fca0e398 100644 --- a/.github/actions/setup-bench/action.yml +++ b/.github/actions/setup-bench/action.yml @@ -84,6 +84,10 @@ runs: # buzz lists frappe/payments in required_apps, so it must precede install-app. bench get-app --skip-assets https://github.com/frappe/payments + # Test-only: buzz/tests/factories imports frappe_factory_bot. No install-app — + # the app ships no DocTypes, and get-app already pip-installs it into the env. + bench get-app --skip-assets https://github.com/harshtandiya/frappe_factory_bot + if [ -n "$EXTRA_APP_URL" ]; then bench get-app --skip-assets "$EXTRA_APP_URL" fi diff --git a/.gitignore b/.gitignore index 97715e66..8f05b25f 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,4 @@ plans/ # Claude Code local state .claude/worktrees/ .claude/settings.local.json +context_local.md diff --git a/buzz/api/booking/test_booking.py b/buzz/api/booking/test_booking.py index bdf44e25..a2febc9d 100644 --- a/buzz/api/booking/test_booking.py +++ b/buzz/api/booking/test_booking.py @@ -12,8 +12,12 @@ ) from buzz.api.booking.exceptions import AddOnNotForEvent, InvalidAddOnValue, RegistrationsClosed from buzz.api.booking.schemas import BookingRequest -from buzz.api.forms.test_forms import ensure_prompt_named_record -from buzz.events.doctype.buzz_team.test_buzz_team import create_owned_team, create_user +from buzz.tests.factories import ( + BuzzEventFactory, + EventTicketTypeFactory, + TicketAddOnFactory, + UserFactory, +) BOOKER = "booking-owner@example.com" OUTSIDER = "booking-outsider@example.com" @@ -71,25 +75,9 @@ class BookingTestCase(IntegrationTestCase): @classmethod def setUpClass(cls): super().setUpClass() - category = ensure_prompt_named_record("Event Category", "Test Booking Category") - host = ensure_prompt_named_record("Event Host", "Test Booking Host") - owner = create_user("booking-team-owner@example.com", "Booking") - cls.team = create_owned_team(f"Booking Test Team {frappe.generate_hash(length=6)}", owner) - cls.event = frappe.get_doc( - { - "doctype": "Buzz Event", - "title": f"Booking Test Event {frappe.generate_hash(length=6)}", - "team": cls.team, - "start_date": "2030-01-01", - "end_date": "2030-01-01", - "start_time": "10:00:00", - "end_time": "18:00:00", - "medium": "Online", - "category": category, - "host": host, - "is_published": 1, - } - ).insert(ignore_permissions=True) + cls.booker = UserFactory.create_once(BOOKER).name + cls.outsider = UserFactory.create_once(OUTSIDER).name + cls.event = BuzzEventFactory.create() cls.event.reload() def setUp(self): @@ -97,15 +85,7 @@ def setUp(self): frappe.clear_messages() self.addCleanup(frappe.clear_document_cache, "Buzz Event", self.event.name) self.set_event({"is_published": 1, "registrations_close_at": None, "allow_guest_booking": 0}) - self.free_ticket_type = frappe.get_doc( - { - "doctype": "Event Ticket Type", - "event": self.event.name, - "title": f"Booking Free {frappe.generate_hash(length=6)}", - "price": 0, - "is_published": 1, - } - ).insert(ignore_permissions=True) + self.free_ticket_type = EventTicketTypeFactory.create(event=self.event.name) def set_event(self, values): frappe.db.set_value("Buzz Event", self.event.name, values) @@ -214,15 +194,7 @@ def test_closed_registrations_are_refused(self): self.assertEqual(frappe.local.message_log[-1]["title"], "Registrations Closed") def make_paid_request(self, **overrides): - paid_ticket_type = frappe.get_doc( - { - "doctype": "Event Ticket Type", - "event": self.event.name, - "title": f"Booking Paid {frappe.generate_hash(length=6)}", - "price": 100, - "is_published": 1, - } - ).insert(ignore_permissions=True) + paid_ticket_type = EventTicketTypeFactory.create("paid", event=self.event.name) return self.booking_request( attendees=[ { @@ -277,10 +249,6 @@ def test_offline_booking_acknowledges_then_confirms(self): """Offline is a two-stage conversation: an acknowledgement while the payment is unverified, the existing confirmation only once an approval submits the booking.""" self.set_event({"send_ticket_email": 0}) - if not frappe.db.exists("User", BOOKER): - frappe.get_doc( - {"doctype": "User", "email": BOOKER, "first_name": "Booking", "send_welcome_email": 0} - ).insert(ignore_permissions=True) frappe.get_doc( { "doctype": "Offline Payment Method", @@ -291,7 +259,7 @@ def test_offline_booking_acknowledges_then_confirms(self): ).insert(ignore_permissions=True) request = self.make_paid_request(is_offline=True) - frappe.set_user(BOOKER) + frappe.set_user(self.booker) self.addCleanup(frappe.set_user, "Administrator") with patch("frappe.sendmail") as sendmail: @@ -299,7 +267,7 @@ def test_offline_booking_acknowledges_then_confirms(self): sendmail.assert_called_once() self.assertEqual(sendmail.call_args[1]["template"], "offline_booking_acknowledgement") - self.assertIn(BOOKER, sendmail.call_args[1]["recipients"]) + self.assertIn(self.booker, sendmail.call_args[1]["recipients"]) self.assertFalse(frappe.db.exists("Event Ticket", {"booking": booking_name})) frappe.set_user("Administrator") @@ -314,19 +282,9 @@ class TestBookingAddOnPricing(BookingTestCase): """The add-on price is server-authoritative: it comes from the Ticket Add-on catalog, never from the booking payload. A guest who names their own price must be ignored.""" - ADD_ON_PRICE = 500 - def setUp(self): super().setUp() - self.add_on = frappe.get_doc( - { - "doctype": "Ticket Add-on", - "event": self.event.name, - "title": f"Meal {frappe.generate_hash(length=6)}", - "price": self.ADD_ON_PRICE, - "enabled": 1, - } - ).insert(ignore_permissions=True) + self.add_on = TicketAddOnFactory.create("paid", event=self.event.name) def book_with_add_on(self, add_on_row): attendees = [ @@ -346,13 +304,13 @@ def book_with_add_on(self, add_on_row): def test_client_supplied_price_is_ignored(self): # The exploit payload: a Rs 500 add-on booked for Rs 1. booking = self.book_with_add_on({"add_on": self.add_on.name, "value": "Veg", "price": 1}) - self.assertEqual(booking.attendees[0].add_on_total, self.ADD_ON_PRICE) - self.assertEqual(booking.total_amount, self.ADD_ON_PRICE) + self.assertEqual(booking.attendees[0].add_on_total, self.add_on.price) + self.assertEqual(booking.total_amount, self.add_on.price) def test_price_is_charged_when_omitted(self): # The legitimate payload carries no price; the catalog price must still be charged. booking = self.book_with_add_on({"add_on": self.add_on.name, "value": "Veg"}) - self.assertEqual(booking.attendees[0].add_on_total, self.ADD_ON_PRICE) + self.assertEqual(booking.attendees[0].add_on_total, self.add_on.price) class TestBookingSelectionValidation(BookingTestCase): @@ -361,35 +319,14 @@ class TestBookingSelectionValidation(BookingTestCase): def setUp(self): super().setUp() - self.add_on = frappe.get_doc( - { - "doctype": "Ticket Add-on", - "event": self.event.name, - "title": f"Meal {frappe.generate_hash(length=6)}", - "price": 500, - "enabled": 1, - "user_selects_option": 1, - "options": "Vegetarian meal\nNon-veg", - } - ).insert(ignore_permissions=True) + self.add_on = TicketAddOnFactory.create( + "paid", + event=self.event.name, + user_selects_option=1, + options="Vegetarian meal\nNon-veg", + ) - foreign_owner = create_user("booking-foreign-owner@example.com", "Foreign") - foreign_team = create_owned_team(f"Foreign Team {frappe.generate_hash(length=6)}", foreign_owner) - self.foreign_event = frappe.get_doc( - { - "doctype": "Buzz Event", - "title": f"Foreign Event {frappe.generate_hash(length=6)}", - "team": foreign_team, - "start_date": "2030-01-01", - "end_date": "2030-01-01", - "start_time": "10:00:00", - "end_time": "18:00:00", - "medium": "Online", - "category": self.event.category, - "host": self.event.host, - "is_published": 1, - } - ).insert(ignore_permissions=True) + self.foreign_event = BuzzEventFactory.create(category=self.event.category, host=self.event.host) def attendee(self, **overrides): row = { @@ -405,30 +342,14 @@ def book(self, attendees): process_booking(self.booking_request(attendees=attendees)) def test_ticket_type_from_another_event_is_refused(self): - foreign_ticket_type = frappe.get_doc( - { - "doctype": "Event Ticket Type", - "event": self.foreign_event.name, - "title": f"Foreign Ticket {frappe.generate_hash(length=6)}", - "price": 0, - "is_published": 1, - } - ).insert(ignore_permissions=True) + foreign_ticket_type = EventTicketTypeFactory.create(event=self.foreign_event.name) with self.assertRaises(frappe.ValidationError): self.book([self.attendee(ticket_type=str(foreign_ticket_type.name))]) self.assertIn("not available for this event", frappe.local.message_log[-1]["message"]) def test_add_on_from_another_event_is_refused(self): - foreign_add_on = frappe.get_doc( - { - "doctype": "Ticket Add-on", - "event": self.foreign_event.name, - "title": f"Foreign Meal {frappe.generate_hash(length=6)}", - "price": 500, - "enabled": 1, - } - ).insert(ignore_permissions=True) + foreign_add_on = TicketAddOnFactory.create("paid", event=self.foreign_event.name) with self.assertRaises(AddOnNotForEvent): self.book([self.attendee(add_ons=[{"add_on": foreign_add_on.name, "value": True}])]) @@ -518,14 +439,6 @@ def test_invalid_attendee_level_phone_is_still_refused(self): class TestGetBookingDetails(BookingTestCase): - def setUp(self): - super().setUp() - for email, first_name in ((BOOKER, "Booking"), (OUTSIDER, "Outsider")): - if not frappe.db.exists("User", email): - frappe.get_doc( - {"doctype": "User", "email": email, "first_name": first_name, "send_welcome_email": 0} - ).insert(ignore_permissions=True) - def make_booking_for(self, user): frappe.set_user(user) try: @@ -544,20 +457,20 @@ def test_shape(self): self.assertEqual(payload["cancelled_tickets"], []) def test_the_booker_reads_their_own_booking(self): - booking_name = self.make_booking_for(BOOKER) - frappe.set_user(BOOKER) + booking_name = self.make_booking_for(self.booker) + frappe.set_user(self.booker) self.assertEqual(get_booking_details(booking_name).doc.name, booking_name) def test_another_user_cannot_read_the_booking(self): - booking_name = self.make_booking_for(BOOKER) - frappe.set_user(OUTSIDER) + booking_name = self.make_booking_for(self.booker) + frappe.set_user(self.outsider) with self.assertRaises(frappe.PermissionError): get_booking_details(booking_name) def test_a_privileged_user_may_read_any_booking(self): - booking_name = self.make_booking_for(BOOKER) + booking_name = self.make_booking_for(self.booker) self.assertEqual(get_booking_details(booking_name).doc.name, booking_name) diff --git a/buzz/tests/__init__.py b/buzz/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/buzz/tests/factories/__init__.py b/buzz/tests/factories/__init__.py new file mode 100644 index 00000000..b861a386 --- /dev/null +++ b/buzz/tests/factories/__init__.py @@ -0,0 +1,17 @@ +from buzz.tests.factories.buzz_event_factory import BuzzEventFactory +from buzz.tests.factories.buzz_team_factory import BuzzTeamFactory +from buzz.tests.factories.event_category_factory import EventCategoryFactory +from buzz.tests.factories.event_host_factory import EventHostFactory +from buzz.tests.factories.event_ticket_type_factory import EventTicketTypeFactory +from buzz.tests.factories.ticket_add_on_factory import TicketAddOnFactory +from buzz.tests.factories.user_factory import UserFactory + +__all__ = [ + "BuzzEventFactory", + "BuzzTeamFactory", + "EventCategoryFactory", + "EventHostFactory", + "EventTicketTypeFactory", + "TicketAddOnFactory", + "UserFactory", +] diff --git a/buzz/tests/factories/buzz_event_factory.py b/buzz/tests/factories/buzz_event_factory.py new file mode 100644 index 00000000..da966ed2 --- /dev/null +++ b/buzz/tests/factories/buzz_event_factory.py @@ -0,0 +1,40 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.events.doctype.buzz_event.buzz_event import BuzzEvent + +_fake = Faker() + + +class BuzzEventFactory(BaseFactory[BuzzEvent]): + """ + Builds a published online event with a fresh team, category and host. Pass + `team=`, `category=` or `host=` to reuse existing records instead. + + `route` is left unset on purpose — `validate_route` derives it from the + unique title and deduplicates it. + """ + + doctype = "Buzz Event" + + @property + def default_attributes(self) -> dict[str, Any]: + from buzz.tests.factories.buzz_team_factory import BuzzTeamFactory + from buzz.tests.factories.event_category_factory import EventCategoryFactory + from buzz.tests.factories.event_host_factory import EventHostFactory + + team = self.overrides.get("team") or BuzzTeamFactory.create_owned_by().name + return { + "title": f"Event {_fake.unique.catch_phrase()}", + "team": team, + "category": self.overrides.get("category") or EventCategoryFactory.create().name, + "host": self.overrides.get("host") or EventHostFactory.create(team=team).name, + "start_date": "2030-01-01", + "end_date": "2030-01-01", + "start_time": "10:00:00", + "end_time": "18:00:00", + "medium": "Online", + "is_published": 1, + } diff --git a/buzz/tests/factories/buzz_team_factory.py b/buzz/tests/factories/buzz_team_factory.py new file mode 100644 index 00000000..a11c2f70 --- /dev/null +++ b/buzz/tests/factories/buzz_team_factory.py @@ -0,0 +1,37 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.events.doctype.buzz_team.buzz_team import BuzzTeam + +_fake = Faker() + +# One reusable owner for teams built as a link target. A fresh user per team runs +# the suite into Frappe's User creation throttle. +LINK_TEAM_OWNER = "factory-team-owner@example.com" + + +class BuzzTeamFactory(BaseFactory[BuzzTeam]): + doctype = "Buzz Team" + + @classmethod + def create_owned_by(cls, user: str | None = None, **overrides: Any) -> BuzzTeam: + """ + Give the team an Owner membership for `user`, defaulting to a shared + throwaway owner. + + Prefer this over a bare `create()`: a plain insert makes the session user the + Owner, and an Administrator-owned team leaks into + `create_default_team_for("Administrator")` for every later run on the site. + + `owner_user` is a flag, and flags only reach the document through overrides. + """ + from buzz.tests.factories.user_factory import UserFactory + + owner = user or UserFactory.create_once(LINK_TEAM_OWNER).name + return cls.create(flags={"owner_user": owner}, **overrides) + + @property + def default_attributes(self) -> dict[str, Any]: + return {"team_name": f"{_fake.unique.word().capitalize()} Team"} diff --git a/buzz/tests/factories/event_category_factory.py b/buzz/tests/factories/event_category_factory.py new file mode 100644 index 00000000..7faab653 --- /dev/null +++ b/buzz/tests/factories/event_category_factory.py @@ -0,0 +1,17 @@ +from typing import Any + +import frappe +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.events.doctype.event_category.event_category import EventCategory + + +class EventCategoryFactory(BaseFactory[EventCategory]): + doctype = "Event Category" + + @property + def default_attributes(self) -> dict[str, Any]: + # autoname is "prompt": the name has to come in with the attributes, and it is + # the primary key. Faker's `unique` only dedupes within a process, and these + # rows outlive a run, so the suffix is a hash. + return {"name": f"Category {frappe.generate_hash(length=8)}"} diff --git a/buzz/tests/factories/event_host_factory.py b/buzz/tests/factories/event_host_factory.py new file mode 100644 index 00000000..2bb3d918 --- /dev/null +++ b/buzz/tests/factories/event_host_factory.py @@ -0,0 +1,21 @@ +from typing import Any + +import frappe +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.events.doctype.event_host.event_host import EventHost + + +class EventHostFactory(BaseFactory[EventHost]): + doctype = "Event Host" + + @property + def default_attributes(self) -> dict[str, Any]: + from buzz.tests.factories.buzz_team_factory import BuzzTeamFactory + + # autoname is "prompt": the name is the primary key, and these rows outlive a + # run, so the suffix is a hash rather than Faker's per-process `unique`. + return { + "name": f"Host {frappe.generate_hash(length=8)}", + "team": self.overrides.get("team") or BuzzTeamFactory.create_owned_by().name, + } diff --git a/buzz/tests/factories/event_ticket_type_factory.py b/buzz/tests/factories/event_ticket_type_factory.py new file mode 100644 index 00000000..4bf02441 --- /dev/null +++ b/buzz/tests/factories/event_ticket_type_factory.py @@ -0,0 +1,29 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.ticketing.doctype.event_ticket_type.event_ticket_type import EventTicketType + +_fake = Faker() + + +class EventTicketTypeFactory(BaseFactory[EventTicketType]): + """Free and published by default. `currency` falls back to the field default (INR).""" + + doctype = "Event Ticket Type" + + @property + def default_attributes(self) -> dict[str, Any]: + from buzz.tests.factories.buzz_event_factory import BuzzEventFactory + + return { + "event": self.overrides.get("event") or BuzzEventFactory.create().name, + "title": f"Ticket {_fake.unique.word().capitalize()}", + "price": 0, + "is_published": 1, + } + + @property + def paid(self) -> dict[str, Any]: + return {"price": 500} diff --git a/buzz/tests/factories/ticket_add_on_factory.py b/buzz/tests/factories/ticket_add_on_factory.py new file mode 100644 index 00000000..be8f319c --- /dev/null +++ b/buzz/tests/factories/ticket_add_on_factory.py @@ -0,0 +1,25 @@ +from typing import Any + +import frappe +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from buzz.ticketing.doctype.ticket_add_on.ticket_add_on import TicketAddon + + +class TicketAddOnFactory(BaseFactory[TicketAddon]): + """Free and enabled by default — `enabled` and `currency` come from the DocType.""" + + doctype = "Ticket Add-on" + + @property + def default_attributes(self) -> dict[str, Any]: + from buzz.tests.factories.buzz_event_factory import BuzzEventFactory + + return { + "event": self.overrides.get("event") or BuzzEventFactory.create().name, + "title": f"Add-on {frappe.generate_hash(length=6)}", + } + + @property + def paid(self) -> dict[str, Any]: + return {"price": 500} diff --git a/buzz/tests/factories/user_factory.py b/buzz/tests/factories/user_factory.py new file mode 100644 index 00000000..8a88567d --- /dev/null +++ b/buzz/tests/factories/user_factory.py @@ -0,0 +1,36 @@ +from typing import Any + +import frappe +from faker import Faker +from frappe.core.doctype.user.user import User +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +_fake = Faker() + + +class UserFactory(BaseFactory[User]): + doctype = "User" + + @classmethod + def create_once(cls, email: str, **overrides: Any) -> User: + """ + Reuse the user at `email` if the site already has one. + + Frappe throttles User creation at `throttle_user_limit` (60) per hour + (`User.throttle_user_creation`), and test users are not rolled back, so a + full suite run that mints a fresh user for every fixture trips it. Use this + wherever the identity is fixed and only one record is ever wanted; use + `create()` when the test needs a distinct user. + """ + if frappe.db.exists("User", email): + return frappe.get_doc("User", email) + return cls.create(email=email, **overrides) + + @property + def default_attributes(self) -> dict[str, Any]: + return { + "email": _fake.unique.email(), + "first_name": _fake.first_name(), + "last_name": _fake.last_name(), + "send_welcome_email": 0, + } From 046bcb84fc594619ce0c7025c928a6e2eb144111 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Fri, 28 Aug 2026 17:37:38 +0530 Subject: [PATCH 2/3] docs: update testing instructions in CLAUDE.md --- CLAUDE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 4313e46c..49c88edd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,6 +56,16 @@ Create specs in `specs/`. Maintain a `PROGRESS.md` file to track progress of imp ## Testing +Backend tests run through Frappe's runner: + +```bash +bench --site buzz.localhost run-tests --app buzz +bench --site buzz.localhost run-tests --module buzz.api.booking.test_booking +``` + +Build test fixtures with the factories in `buzz/tests/factories/` (powered by +`frappe_factory_bot`), never `frappe.get_doc({...}).insert()`. See the `writing-tests` skill. + Use agent-browser for quick manual e2e checks. Automated e2e uses Playwright (root `package.json`, specs in `e2e/`). From d54eecae5ad05c6b812d7417574d6dd748fb7719 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Fri, 28 Aug 2026 17:39:33 +0530 Subject: [PATCH 3/3] ci: pin frappe_factory_bot to a reviewed commit The CI bench setup pulled the factory app from its default branch, so upstream drift could break setup or factory imports without any Buzz change, and an unreviewed revision would run in CI. The repo carries no tags and `bench get-app --branch` forwards to `git clone --branch`, which rejects a SHA, so clone and check out the commit first and hand bench the local path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012BsQrw3T6rYENg46pDJivL --- .github/actions/setup-bench/action.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/actions/setup-bench/action.yml b/.github/actions/setup-bench/action.yml index fca0e398..cea5c9d2 100644 --- a/.github/actions/setup-bench/action.yml +++ b/.github/actions/setup-bench/action.yml @@ -86,7 +86,11 @@ runs: # Test-only: buzz/tests/factories imports frappe_factory_bot. No install-app — # the app ships no DocTypes, and get-app already pip-installs it into the env. - bench get-app --skip-assets https://github.com/harshtandiya/frappe_factory_bot + # Pinned to a reviewed commit: the repo has no tags and `bench get-app --branch` + # rejects a SHA, so clone and check out before handing bench the local path. + git clone https://github.com/harshtandiya/frappe_factory_bot /tmp/frappe_factory_bot + git -C /tmp/frappe_factory_bot checkout 48904d0df778513934e92a4e7663c74bfe31bc56 + bench get-app --skip-assets /tmp/frappe_factory_bot if [ -n "$EXTRA_APP_URL" ]; then bench get-app --skip-assets "$EXTRA_APP_URL"