test: build booking fixtures with frappe_factory_bot factories - #393
Open
harshtandiya wants to merge 3 commits into
Open
test: build booking fixtures with frappe_factory_bot factories#393harshtandiya wants to merge 3 commits into
harshtandiya wants to merge 3 commits into
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016dbs2RcPoTwFTM2JTE386q
Contributor
Greptile SummaryThis PR introduces reusable
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| .github/actions/setup-bench/action.yml | The previously mutable test dependency is now cloned and checked out at an immutable commit before bench installation. |
| buzz/api/booking/test_booking.py | Booking test fixture setup is consolidated onto the new factories while preserving the tested booking and permission scenarios. |
| buzz/tests/factories/buzz_event_factory.py | Adds a published online-event factory that creates or reuses the required team, category, and host associations. |
| buzz/tests/factories/buzz_team_factory.py | Adds team fixtures with explicit reusable ownership to avoid persistent Administrator-owned test teams. |
| buzz/tests/factories/user_factory.py | Adds reusable fixed-identity users to avoid repeated creation and Frappe throttling. |
| buzz/tests/factories/event_category_factory.py | Adds collision-resistant prompt-named category fixtures. |
| buzz/tests/factories/event_host_factory.py | Adds collision-resistant host fixtures associated with an appropriate team. |
| buzz/tests/factories/event_ticket_type_factory.py | Adds free and paid ticket-type fixtures with optional event reuse. |
| buzz/tests/factories/ticket_add_on_factory.py | Adds free and paid add-on fixtures with optional event reuse. |
Reviews (3): Last reviewed commit: "ci: pin frappe_factory_bot to a reviewed..." | Re-trigger Greptile
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012BsQrw3T6rYENg46pDJivL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Fixture setup in the backend suite is duplicated everywhere —
test_buzz_coupon_code.pyhas 74 raw
frappe.get_doc({...})call sites,test_event_booking.py68 — and an informalfactory layer has already grown by accident.
test_buzz_team.pyis imported by ten othertest files for
create_user/create_owned_team, andpayload_foris a per-doctypedefault_attributestable in all but name.This adopts
frappe_factory_botproperly, on one module first.buzz/api/booking/test_booking.pyis the tracer bullet: its
setUpClassbuilds the whole association chain, so converting itexercises every factory added here. −118/+31 lines in that file.
buzz/tests/factories/— User, Buzz Team, Event Category, Event Host, Buzz Event,Event Ticket Type, Ticket Add-on.
bench get-appinsetup-bench. Noinstall-app(it ships noDocTypes, only a Python import), and deliberately not in
required_apps— productionBuzz should not carry a test library.
writing-testsskill documenting the conventions and the traps below.Three Frappe-level traps, all found the hard way, all now in the skill:
later run on that site —
create_default_team_fortakes the first enabled Ownermembership, and
process_bookingcommits, so the row survives rollback.setup_test_records()then fails with
Venue Test Venue belongs to another team.HenceBuzzTeamFactory.create_owned_by().User.throttle_user_creationthrows past 60 new users an hour. Test users are not rolledback, so minting a fresh one per fixture trips it after a couple of runs. Hence
UserFactory.create_once()for fixed identities.uniqueonly dedupes within a process. Prompt-autonamed rows are the primary keyand outlive the run, so Event Category and Event Host suffix with
frappe.generate_hash.Not changed:
frappe_factory_botitself needed nothing — theflagspassthrough alreadycarries both
owner_userandignore_permissions. Event Booking, Offline Payment Method andBuzz Coupon Code get no factory yet; nothing here would exercise them, so they land with the
batch that does. The old helpers in
test_buzz_team.py/test_forms.pystay — twelve filesstill import them, and they retire as each module converts.
Testing
bench --site testbuzz.localhost run-tests --app buzz— 670 tests, 0 failures, green onrepeated back-to-back runs (the cross-run collisions above only surface on the second and
third run). Two consecutive full runs created 3 users total, so CI's fresh site is nowhere
near the throttle.
🤖 Generated with Claude Code
https://claude.ai/code/session_016dbs2RcPoTwFTM2JTE386q