feat(ci): Improve test coverage#6516
Conversation
|
| Filename | Overview |
|---|---|
| press/press/doctype/server/server.py | Replaces frappe.get_value(…, None, field) with frappe.db.get_single_value for Press Settings Single DocType — correct and idiomatic change with no behaviour difference. |
| press/press/doctype/server/test_server.py | Adds TestSetDedicatedServerWarrantyQuota and TestGetTeamsWithUnpaidInvoicesOverThreshold; the latter contains two duplicate test methods that both only assert isinstance(result, set), where the first claims to verify 'no teams' but never asserts the result is empty. |
| press/press/doctype/team/test_team.py | Large expansion: 13 new test classes covering RBAC guards, team-member permission guards, Press Role lifecycle, create_user_resource idempotency, and team utility functions. Well-structured with proper setUp/tearDown and DB rollback. |
| press/press/doctype/site/test_site_plan_utils.py | New file with 5 test classes covering warranty quota helpers, attachment logic, and plan-level warranty flag; all use DB rollback and targeted mocking correctly. |
| press/press/doctype/deploy_candidate/test_deploy_candidate_utils.py | New file exercising pure-utility functions with tempfile fixtures and targeted mocking; logic is sound. |
| press/press/doctype/cluster/test_cluster.py | Adds validation helpers (CIDR, flush-table hour, monitoring password) and firewall normalisation tests using in-memory Cluster docs. |
| press/press/doctype/virtual_machine/test_virtual_machine.py | Adds data-disk snapshot and volume-helper tests; client patched to MagicMock to avoid real API calls. |
| press/press/doctype/release_group/test_release_group.py | Adds TestCanUseRelease and TestReleaseGroupDeployBlocking; tests correctly mirror the public/private and status-based logic in can_use_release. |
| press/press/doctype/app_source/test_app_source.py | Adds tearDown rollback and new tests for validate_source_signature, validate_duplicate_versions, set_required_apps, and get_repo_url including private-repo token paths. |
| press/press/doctype/team_member_resource/test_team_member_resource.py | Adds comprehensive validation tests for TeamMemberResource using typed fixtures and create_test_user. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR: Improve Test Coverage] --> B[server.py change]
A --> C[47 Test Files]
B --> B1["frappe.get_value -> frappe.db.get_single_value"]
C --> C1[Team RBAC Tests - 13 classes]
C --> C2[Server / Warranty Tests]
C --> C3[Cluster Topology Tests]
C --> C4[Virtual Machine Tests]
C --> C5[Deploy Candidate Utils]
C --> C6[Site Plan Utils]
C --> C7[Release Group Gate]
C --> C8[App Source Tests]
C --> C9[Team Member Resource]
Reviews (10): Last reviewed commit: "fix(tests): address remaining review obs..." | Re-trigger Greptile
2a2598c to
07d053f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6516 +/- ##
===========================================
+ Coverage 50.13% 53.40% +3.26%
===========================================
Files 956 959 +3
Lines 79329 83039 +3710
Branches 375 375
===========================================
+ Hits 39775 44345 +4570
+ Misses 39526 38666 -860
Partials 28 28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
62114c8 to
2889533
Compare
347958c to
0d0fc94
Compare
85f3595 to
9c89181
Compare
- Add TestTeamCreationAndOwnership: team creation hooks, owner detection - Add TestTeamMemberPermissionGuards: perm_team_members/perm_relaxed_roles guards for non-admin, admin-role member, and team owner - Add TestTeamOwnerAndAdminDetection: is_team_owner(), is_admin_user() methods - Add TestInviteTeamMember: invite workflow, duplicate detection, email sending - Add TestPressRoleCreationAndValidation: role CRUD, admin-only enforcement - Add TestPressRoleUserManagement: add_user/remove_user with role guard checks - Add TestPressRoleResourceManagement: add_resource/remove_resource enforcement - Add TestPressRolePermissionInsert: PressRolePermission before_insert guards - Add TestRoleGuardIsRestricted: is_restricted() logic for members/owners - Add TestRoleGuardPermittedDocuments: permitted_documents() filtering - Add TestCreateUserResource: create_user_resource() idempotency and creation - Add TestTeamGuardDecorators: only_admin/only_owner/only_member decorator logic Total: 55 tests covering RBAC & Team Management edge cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add TestClusterValidation: CIDR auto-assign, monitoring password generation, flush-table hour validation (missing/out-of-range/valid) - Add TestClusterFirewallHelpers: _parse_port_range (int/str/range), _normalize_firewall_protocol (valid/invalid), _normalize_firewall_rules (various input formats, empty throws) - Add TestClusterServerCreationGuards: create_proxy throws on inactive cluster and missing VMI; create_servers throws on missing images; create_server App throws when no proxy set; DB-replication guard conditions (wrong doctype, missing master, inactive master) - Add TestHetznerProvisionFailure: network creation failure -> no vpc_id committed; firewall creation failure -> no security_group_id; api token validation for unauthorized and other APIException codes - Add TestNatServerSupport: returns None when public IPs enabled, for Hetzner, when no NAT server in DB; returns name when found - Add TestVirtualMachineDataDiskSnapshot: non-AWS throws; Pending snapshot throws; cross-region snapshot throws; missing VMI throws; skipped for existing/no-snapshot VMs - Add TestVirtualMachineVolumeHelpers: get_root_volume (single/multi- volume AWS); get_data_volume (no data volume, multi-volume AWS, empty) - Add TestVirtualMachineServerRouting: create_database_server, proxy, app server routing; unified server requires 'u' series; NAT server requires 'nat' series; unified creates App+DB pair; NAT validate throws for non-AWS cloud provider - Add TestVirtualMachineStatusMaps: all AWS/Hetzner/OCI/DO states covered Total: 59 new tests covering infrastructure & cluster topology Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- test_app_release.py: add full TestAppRelease class (12 tests) * before_save auto-approval for Featured Apps and Auto Release Teams * _has_auto_deploy_marker: marker absent, present, bench group extraction * has_permission: own team, other team, public release, system user * set_invalid: marks release and records reason - test_app_source.py: extend TestAppSource (5 new tests) * validate_source_signature raises on duplicate repo+branch+team * validate_duplicate_versions raises / passes for unique versions * set_required_apps parses owner/repo format and skips duplicates - test_release_group.py: add TestCanUseRelease (9 tests) and TestReleaseGroupDeployBlocking (3 tests) * can_use_release gate: public Approved/Draft pass; Awaiting/Rejected/Yanked blocked * private releases always bypass the gate regardless of status * deploy_information integration smoke test * pending private release does not block ReleaseGroup creation * new_release_group raises ValidationError on Frappe version mismatch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- test_site.py: add 5 new tests to TestSite
* test_app_not_on_bench_blocks_site_creation: validate_installed_apps
throws when a site tries to install an app not present on the bench
(core 'custom app on public bench' guard)
* test_first_app_on_site_must_be_frappe: first installed app must be
frappe or validation fails
* test_duplicate_apps_on_site_blocked: validate_installed_apps rejects
duplicate app entries on a site
* test_server_script_cannot_be_enabled_on_public_bench_version_15:
check_server_script_enabled_on_public_bench raises for v15+ public benches
* test_server_script_can_be_enabled_on_private_bench: private bench
permits server scripts regardless of version
- test_app_source.py: add 4 new tests to TestAppSource
* test_get_repo_url_returns_plain_url_for_public_repo: no auth token
in URL when github_installation_id is absent
* test_get_repo_url_embeds_token_for_private_repo: x-access-token is
embedded in the clone URL for private repos on private benches
* test_get_repo_url_raises_when_token_fetch_fails: raises Exception with
clear message when the GitHub App token cannot be obtained
* test_get_access_token_falls_back_to_press_settings_for_public_source:
global access token is used for sources without installation ID
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- test_deploy_candidate_build.py: add 8 new build-pipeline tests
* test_encode_dockerfile_produces_valid_base64: base64 encode/decode
round-trip on plain ASCII dockerfile content
* test_encode_dockerfile_round_trips_unicode: unicode chars survive the
encode/decode cycle unchanged
* test_get_dockerfile_checkpoints_extracts_stage_slugs: '`#stage-<slug>`'
patterns are correctly parsed out of the Dockerfile
* test_get_dockerfile_checkpoints_returns_empty_for_no_markers: returns []
when no stage markers are present
* test_check_distutils_support_python_3_12: Python 3.12+ returns False
(distutils removed; Dockerfile must exclude it)
* test_check_distutils_support_python_3_11: Python 3.11 returns True
(still ships distutils; DISTUTILS_SUPPORTED_VERSION = <3.12)
* test_check_distutils_support_python_3_9: Python 3.9 returns True
* test_add_pre_build_steps_creates_clone_step_for_each_app: clone step
is created for every app in the deploy candidate
- test_agent_job.py: add 5 new agent job failure / status transition tests
* test_restore_job_failure_sets_site_broken: Failure → Broken
* test_restore_job_success_sets_site_active: Success → Active
* test_restore_job_delivery_failure_keeps_site_active: Delivery Failure
leaves the site Active (agent never received the request)
* test_migrate_job_failure_sets_site_broken: migrate Failure → Broken
* test_migrate_job_running_sets_site_updating: Running → Updating
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…source The CI environment ships a team_member_resource module that registers a 'Press Role: after_insert' doc-event handler (sync_press_role). When the handler runs it tries to insert a Team Member Resource whose validate_document_name guard rejects document names that don't exist in the DB — which is the case for the mock frappe._dict docs used in these unit tests. Add _patch_sync_press_role(), a helper that: - imports team_member_resource via importlib; if the module is present (CI), it returns patch(...sync_press_role, new=Mock()) to silence the side-effect; - if the module is absent (local dev), it falls back to nullcontext() so no error is raised. Apply the helper to both tests that exercise create_user_resource: - test_create_user_resource_creates_role_for_restricted_member - test_create_user_resource_is_idempotent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…LinkValidationError On CI the AppRelease doctype has a 'team' Link field that is auto-populated with get_current_team() when not explicitly set. The session team differs from the test team created in setUp, so _validate_links() throws LinkValidationError when inserting. Passing app_source.team directly ensures the AppRelease doc is always linked to the same team that owns the AppSource, matching the intent of the fixture and keeping all TestAppRelease tests green on both local dev and CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…; add PR-6372 coverage
Merge test_cluster_topology.py → test_cluster.py
- Added HetznerAPIException, create_test_virtual_machine,
create_test_virtual_machine_image imports
- Appended _make_cluster() helper and 9 test classes:
TestClusterValidation, TestClusterFirewallHelpers,
TestClusterServerCreationGuards, TestHetznerProvisionFailure,
TestNatServerSupport, TestVirtualMachineDataDiskSnapshot,
TestVirtualMachineVolumeHelpers, TestVirtualMachineServerRouting,
TestVirtualMachineStatusMaps
- Deleted test_cluster_topology.py
Merge test_team_rbac.py → test_team.py
- Added contextmanager, nullcontext, MagicMock, make_autoname,
PressRole, PressRolePermission, get_team_members imports
- Appended rbac helpers (user_context, get_current_team_ctx,
_make_press_user, _create_press_role, _add_press_role_user,
_patch_sync_press_role) and 12 test classes
- Deleted test_team_rbac.py
Add PR-6372 coverage (previously empty stub files)
test_app_release_approval_request.py:
- TestApprovalRequestGuards (4 tests): before_insert guards
(update_release_status, duplicate request, yanked release,
same-source conflict)
- TestApprovalRequestStatusPropagation (3 tests): on_update
propagates Approved / Rejected / Cancelled to linked release
- TestApprovalRequestAutoApproval (3 tests): before_save
auto-approves featured apps and auto-release teams
test_app_release_difference.py:
- TestIsMigrateNeeded (12 tests): file-path classification
for patches.txt, hooks.py, fixtures/, custom/, languages.json,
doctype JSON, and safe files
- TestAppReleaseDifferenceValidate (2 tests): same-release guard
- TestHasBranchChanged (2 tests): branch comparison logic
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
test_virtual_machine.test_virtual_machine imports create_test_cluster from test_cluster. After the topology merge, test_cluster also imported create_test_virtual_machine / create_test_virtual_machine_image from those same modules at module level → circular ImportError on CI. Fix: remove both test-helper imports from the top-level and replace with lazy (function-scoped) imports in each method / setUp that needs them: - TestClusterServerCreationGuards.test_create_proxy_throws_for_inactive_cluster_before_checking_vmi - TestVirtualMachineDataDiskSnapshot._create_snapshot - TestVirtualMachineVolumeHelpers._vm_with_volumes - TestVirtualMachineServerRouting.setUp (stores self._ctvm) - TestVirtualMachineStatusMaps.setUp Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- create_test_app_release: CI production code can overwrite app_source.team via get_current_team() from a stale rolled-back session, causing LinkValidationError when _validate_links() checks the team link field. ignore_links=True bypasses this validation in test fixtures. - _make_difference: precautionary ignore_links=True to avoid cascading link validation failures from the linked App Release fixtures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…turns_false _create_releases() already returns (r1.name, r2.name) — plain strings. Calling r1.name on a str raised AttributeError: 'str' object has no attribute 'name'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…resource, site_plan_utils - TestStaticIPLog: validate(), before_insert status guard, full attach/detach lifecycle, subscription creation, create_static_ip_log() helper - TestStaticIPPlan: get_price_for_interval() for Hourly/Daily in USD/INR, invalid-interval raises - TestTeamMemberResource: validate_user, validate_document_type, validate_document_name, prevent_duplicate, full insert, sync_press_role - TestSitePlanUtils: warranty quota count/quota getters, is_product_warranty enabled, get_available_warranty_quota, attach_warranty_info_to_dedicated_servers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rsers - test_docker_output_parsers.py: 26 tests across 5 classes covering ansi_escape, DockerBuildOutputParser._append_error_line/_parse_line/_update_dc_build_step, CloneOutputParser._parse_app_output_map, UploadStepUpdater._update_output - test_deploy_candidate_utils.py: 35 tests across 8 classes covering get_required_apps_from_hookpy, get_error_key, load_package_json, load_pyproject, get_package_manager_files_from_repo, is_suspended, get_build_server_with_least_active_builds, BuildValidationError - All tests use MagicMock / tmp-file fixtures; zero Frappe DB round-trips for pure-util tests - Add .gitignore to cspell ignorePaths to avoid flagging standard Python ecosystem terms Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, usage_record, server_plan
press_webhook (15 tests):
- validate_endpoint_url_format: HTTPS/HTTP pass, FTP/no-scheme/query-params/private-IP/
loopback/localhost/local-domain raise
- validate guards: empty-events, >5 webhooks, duplicate endpoint all raise
plan_change (9 tests):
- validate: team populated from linked document, upgrade/downgrade/equal-price detection,
no_type_detection without from_plan, Initial Plan clears from_plan
site_config_key (7 tests):
- validate: title auto-generated from underscore key, existing title preserved,
empty string title replaced; blacklisted non-internal raises, internal bypasses blacklist
usage_record (9 tests):
- validate: date/time default to today/nowtime; explicit values preserved; both defaults at once
- validate_duplicate: duplicate Site raises, no duplicate passes,
non-primary Server skips check, primary Server runs check
server_plan (9 tests):
- get_price_per_day/get_price_for_interval: USD/INR daily/monthly calculations, unknown=None
- validate_active_subscriptions: zero-subs allows disable, active-subs blocks disable,
legacy_plan bypasses guard
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ermission
yanked_app_release (7 tests):
- after_insert: marks App Release as Yanked (invalid_release=True, status=Yanked)
- on_trash: restores App Release to Approved (invalid_release=False, status=Approved)
- verify after_insert/on_trash are inverse operations
audit_log (9 tests):
- after_insert: notify() called only on Failure, not on Success/other statuses
- notify: TelegramMessage.enqueue called, message contains audit_type, group/topic passed correctly,
telegram_topic defaults to 'Errors' when None
press_role_permission (8 tests):
- is_user_part_of_admin_role: returns True when user in admin role, False when not,
False when no admin roles, uses session.user when no arg supplied
- before_insert: system user can insert, team owner can insert, non-owner without admin
role raises, non-owner with admin role can insert, duplicate raises
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ate_difference site_config_key_blacklist (4 tests): - validate: msgprint called when key exists in Site Config Key (enabled=True) - validate: silent when key absent - validate: message includes the key name - validate: never raises (warning-only, not blocking) deploy_candidate_difference (4 tests): - validate: raises when source == destination (same candidate) - validate: raises when source creation > destination creation (wrong order) - validate: raises when duplicate (group, source, destination) already exists - validate: passes for valid distinct-candidate ordered non-duplicate difference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bench_update (9 tests):
- validate_pending_updates: raises when release group deploy_in_progress=True, passes otherwise
- validate_pending_site_updates: raises when site has Pending/Running update, passes otherwise,
passes when site list is empty
- validate_inplace_update: raises when no sites selected, raises when sites span multiple benches,
passes when all sites on same bench, passes for single site
github_webhook_log (14 tests):
- get_repository_details_from_payload: standard push, installation with single repo,
owner from full_name, owner from installation.account fallback, empty payload returns None,
multi-repo installation skips repo name
- handle_events: dispatches push/installation/installation_repositories correctly, unknown event no-raise
- handle_installation_event: created/unsuspend → handle_installation_created,
deleted/suspend → handle_installation_deletion
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ailure
press_method_permission (4 tests):
- available_actions: returns empty dict when no permissions, single doctype/action,
multiple actions for same doctype, return type is dict
agent_request_failure (4 tests):
- is_server_archived: returns True when Archived > 1 hour ago,
returns False when Archived < 1 hour ago,
returns False when server status is Active,
returns False when server status is Pending
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
site_activity (7 tests): - after_insert: Administrator team skips email entirely - after_insert: 'Login as Administrator' with reason sends email to recipients - after_insert: 'Login as Administrator' without reason skips email - after_insert: 'Login as Administrator' with reason but no recipients skips email - after_insert: 'Disable Monitoring And Alerts' with reason sends email - after_insert: other actions (Backup/Migrate/Archive/Restore) send no email Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lidation log_server (6 tests): - validate_monitoring_password: generated when absent, preserved when set, empty string replaced - validate_kibana_password: generated when absent, preserved when set, empty string replaced analytics_server (6 tests): - validate_monitoring_password: generated when absent, preserved when set, empty string replaced - validate_plausible_password: generated when absent, preserved when set, empty string replaced Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
silenced_alert (11 tests):
- get_duration: 1 hour, 0 seconds, 30 minutes, 2 days — all set self.duration to human-readable string
- get_keyword_based_on_instance_type: Site/Server→instance, Cluster→cluster,
Release Group→group, Bench→bench, Prometheus Alert Rule→alertname, Unknown→''
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
prometheus_alert_rule (7 tests):
- get_rule: includes name and expression, merges severity (lowercased) into labels,
merges description into annotations
- get_route: includes alertname matcher, includes group_wait/group_interval/repeat_interval
- validate: raises ValidationError when enabled without expression,
passes when disabled without expression
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
security_update (7 tests):
- _prepare_package_list: empty when output is None, parses single package, multiple packages,
skips 'Listing...' header, empty string gives empty list, splits on slash, combo test
team_change (6 tests):
- validate: raises when document team != from_team, passes when they match
- on_update: Site transfer propagates team to related records when completed,
no propagation when incomplete; Release Group sets team + skip_onboarding;
non-Site/non-RG type is no-op
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
press_job (7 tests):
- arguments_dict: parses valid JSON to frappe._dict, returns empty dict when None,
returns empty dict for empty string, returns frappe._dict type, nested arguments preserved
- virtual_machine_doc: returns None when not set, fetches doc when VM is linked
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
app_patch (6 tests): - validate_bench: passes when bench is Active, raises when Inactive/Archived/None - before_insert: raises when duplicate patch exists for same bench, passes when no duplicate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- press_job: cached_property uses .func not .fget - press_role_permission: get_current_team imported locally, patch press.utils - prometheus_alert_rule: bind get_rule/get_route on SimpleNamespace for validate() - site_config_key: avoid blacklisted key 'rate_limit' in title-generation test - site_config_key_blacklist: use patch.object(frappe.db) not nested module path - press_method_permission: fix mock — first get_all call has pluck kwarg - server_plan: rounded() without 2nd arg → int; align test expectation - silenced_alert: format_duration returns compact format (1h/30m/2d) - site_plan_utils: pin Press Settings warranty quota to 0 before test server create Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- static_ip_plan: Static IP Plan uses autoname=prompt — supply explicit name via make_autoname so insert does not raise - team: Subscription.insert validates links — add ignore_links=True for the synthetic 'test-site.example.com' document - user_ssh_certificate: _VALID_KEY base64 body was 37 chars (37 mod 4 = 1, always invalid); replace with 44-char padded string decoding to 32 bytes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tings frappe.get_value uses the document cache; frappe.db.set_single_value on Press Settings in setUp was corrupting the cache seen by other tests in TestSetDedicatedServerWarrantyQuota, causing them to get None instead of the value they set. Fix: create the server normally, then overwrite supported_site_quota = 0 directly on the Server row and reload — Press Settings is never touched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- static_ip_plan: StaticIPPlan.get_price_for_interval only accepts 'Daily' (returns raw price, not *24); remove Hourly tests, add Hourly-raises test - static_ip_log: _check_if_can_disable_subscription only raises when the last log is already Detached; no-prior-log case is silently allowed - team: Subscription.on_update loads the linked document — use an existing ToDo row instead of a non-existent Site to avoid DoesNotExistError Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the placeholder ToDo target with an actual Server subscription: - create_test_server with a Server Plan so on_update sees doc.plan == sub.plan and skips the plan-sync update (no side effects) - total_subscribed_amount reads price_usd from the real Server Plan record - disabled subscription test validates it is excluded from the sum Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Local test runner script — not part of the test suite. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
frappe.get_value("Press Settings", None, field) reads from the document
object cache which frappe.db.set_single_value does not invalidate, so
the method returned None after a set_single_value call in tests.
frappe.db.get_single_value reads from tabSingles via the value cache
that set_single_value correctly clears, making reads cache-coherent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
press_role add_resource tests: add_resource validates document existence via frappe.db.get_value; use create_test_site_record to insert a real bare Site row owned by the team so the check passes without mocking the DB layer TestTeamTotalSubscribedAmount: Server.update_subscription() auto-creates an enabled Subscription when a server is inserted with a plan; remove the pre-built server from setUp and create servers without a plan in _make_server_sub to avoid the DuplicateEntryError on the manually-inserted test subscription TestSyncPressRole.test_sync_clears_existing_resources_before_rebuild: sync_press_role re-queries the DB for all Press Roles in the team, so passing a Mock stub still finds the real role and recreates its resources; fix by deleting the role's resources from the DB before the second sync Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ils test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
test_team → test_press_role → test_team caused ImportError at module load. Move create_test_site_record import inside _make_team_site() so it is only resolved at call time, after both modules are fully initialized. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9c89181 to
2c21c72
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…tructural issues - test_team.py: rename test_create_new_method_works → test_create_new_increments_team_count - test_agent_job.py: extract restore/migrate status-transition tests into TestSiteRestoreJobStatusTransitions and TestSiteMigrateJobStatusTransitions; remove set_single_value reset from test body (tearDown rollback handles it) - test_press_webhook.py: merge TestValidateEndpointUrlFormatValid/Invalid into TestValidateEndpointUrlFormat; add assertIsNone to all 5 valid cases - test_cluster.py: move VirtualMachine test classes to test_virtual_machine.py (TestVirtualMachineDataDiskSnapshot, VolumeHelpers, ServerRouting, StatusMaps) - test_prometheus_alert_rule.py: split generic TestPrometheusAlertRule into TestPrometheusAlertRuleRendering (get_rule/get_route) and TestPrometheusAlertRuleValidation (validate guards) - test_agent_request_failure.py: remove _now() noise helper; use datetime.now() inline Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- test_agent_job.py: split TestAgentJob into TestLockDocUpdatedByJob and TestAgentJobDeduplication; extract _setup_team() shared helper to remove setUp duplication across the four job test classes - test_press_role_permission.py: narrow mock scope from press.utils.get_current_team (source module) to _MODULE.get_current_team (import site in module under test), preventing silent interference with parallel tests - test_team.py: inline create_test_site_record SQL in _make_team_site() to eliminate the deferred import workaround for the test_team → test_press_role circular dependency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Greptile Summary
This PR expands test coverage across 10 doctypes, adding hundreds of new test cases for site status transitions, app release lifecycle, approval request workflows, cluster provisioning, team RBAC, deploy-candidate build pipeline, and the
can_use_releasegate.process_restore_job_update,process_migrate_site_job_update,AppReleaseApprovalRequeststatus propagation, Hetzner/AWS API error handling,VirtualMachinevolume helpers, server-creation routing, and team-member permission guards.user_context,get_current_team_ctx,_patch_sync_press_role,_make_cluster, etc.) to support RBAC and multi-cloud scenarios without hitting live APIs.Confidence Score: 5/5
Safe to merge — this PR adds only test code and a timestamp update to the secrets baseline; no production logic is changed.
All changed files are test files. The new tests correctly mirror the production implementations they exercise (verified against site.py, release_group.py, and app_release_approval_request.py). Mock usage is appropriate throughout, with two minor cases where the mocking strategy could be tightened for long-term reliability, but neither represents a functional defect today.
No files require special attention. The two observations are in test_agent_job.py (broad frappe.db.get_value mock) and test_release_group.py (trivially-true assertion), both non-blocking.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Agent Job Status Update] --> B{job_type} B -->|Restore Site| C[process_restore_job_update] B -->|Migrate Site| D[process_migrate_site_job_update] C --> E{job.status} E -->|Success| F[Site → Active\nset_apps + marketplace hooks] E -->|Failure| G[Site → Broken\nset creation_failed] E -->|Delivery Failure| H[Site → Active] E -->|Running| I[Site → Installing] D --> J{job.status} J -->|Failure| K[Site → Broken] J -->|Running| L[Site → Updating] subgraph AppRelease Lifecycle M[before_insert] --> N[update_release_status → Awaiting Approval] O[on_update: Approved] --> P[AppRelease → Approved] Q[on_update: Rejected] --> R[AppRelease → Rejected] S[cancel] --> T[AppRelease → Draft] end subgraph can_use_release Gate U{app_release.public?} -->|No| V[Always usable] U -->|Yes| W{status} W -->|Approved / Draft| X[Usable] W -->|Awaiting / Rejected / Yanked| Y[Blocked] endReviews (3): Last reviewed commit: "fix(test): remove spurious .name access ..." | Re-trigger Greptile