Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR plausible#5909

Type: Clean (correct implementation)

Original PR Title: Slurp common test modules into exunit templates
Original PR Description: unknown
Original PR URL: plausible#5909


PR Type

Enhancement


Description

  • Consolidate test module imports into base test case templates

  • Move Plausible.Teams.Test usage into DataCase and ConnCase

  • Remove redundant use Plausible statements from individual test files

  • Simplify test file headers by centralizing common test utilities


Diagram Walkthrough

flowchart LR
  A["Individual Test Files<br/>use Plausible.Teams.Test<br/>use Plausible"] -->|Refactor| B["DataCase & ConnCase<br/>Base Templates"]
  B -->|Include| C["Plausible.Teams.Test"]
  B -->|Include| D["Plausible.TestUtils"]
  B -->|Include| E["Plausible.Test.Support.HTML"]
  C --> F["Cleaner Test Files<br/>Reduced Boilerplate"]
  D --> F
  E --> F
Loading

File Walkthrough

Relevant files
Enhancement
78 files
conn_case.ex
Add common test utilities to ConnCase template                     
+4/-2     
data_case.ex
Add common test utilities to DataCase template                     
+3/-0     
plugins_api_case.ex
Add common test utilities to PluginsAPICase template         
+3/-2     
audit_test.exs
Remove redundant test module imports                                         
+1/-5     
auth_test.exs
Remove redundant test module imports                                         
+0/-1     
verification_test.exs
Remove redundant test module imports                                         
+0/-3     
domains_test.exs
Remove redundant test module imports                                         
+0/-2     
sso_test.exs
Remove redundant test module imports                                         
+0/-2     
user_sessions_test.exs
Remove redundant test module imports                                         
+0/-2     
billing_test.exs
Remove redundant test module imports                                         
+0/-1     
enterprise_plan_test.exs
Remove redundant test module imports                                         
+0/-1     
feature_test.exs
Remove redundant test module imports                                         
+0/-1     
plans_test.exs
Remove redundant test module imports                                         
+0/-1     
quota_test.exs
Remove redundant test module imports                                         
+0/-3     
site_locker_test.exs
Remove redundant test module imports                                         
+0/-1     
cache_test.exs
Remove redundant test module imports                                         
+0/-1     
cache_test_sync.exs
Remove redundant test module imports                                         
+0/-1     
consolidated_view_test.exs
Reorganize test module usage and imports                                 
+2/-3     
enterprise_plan_test.exs
Remove redundant test module imports                                         
+0/-1     
backfill_teams_hourly_request_limit_test.exs
Remove redundant test module imports                                         
+0/-1     
prefix_tracker_script_configuration_id_test.exs
Remove redundant test module imports                                         
+0/-1     
funnels_test.exs
Remove redundant test module imports                                         
+0/-3     
goals_test.exs
Remove redundant test module imports                                         
+0/-2     
help_scout_test.exs
Remove redundant test module imports                                         
+0/-2     
csv_importer_test.exs
Remove redundant test module imports                                         
+0/-2     
google_analytics4_test.exs
Remove redundant test module imports                                         
+0/-1     
imported_test.exs
Remove redundant test module imports                                         
+0/-1     
counters_test.exs
Remove redundant test module imports                                         
+0/-1     
event_telemetry_test.exs
Remove redundant test module imports                                         
+0/-1     
event_test.exs
Remove redundant test module imports                                         
+0/-1     
browserless_config_test.exs
Reorder test module usage statements                                         
+1/-1     
check_test.exs
Remove redundant test module imports                                         
+0/-1     
props_test.exs
Remove redundant test module imports                                         
+0/-1     
purge_test.exs
Remove redundant test module imports                                         
+0/-1     
release_test.exs
Remove redundant test module imports                                         
+0/-1     
cache_test.exs
Remove redundant test module imports                                         
+0/-1     
gate_keeper_test.exs
Remove redundant test module imports                                         
+0/-1     
site_removal_test.exs
Remove redundant test module imports                                         
+0/-1     
sites_test.exs
Remove redundant test module imports                                         
+0/-2     
clickhouse_test.exs
Remove redundant test module imports                                         
+0/-2     
comparisons_test.exs
Remove redundant test module imports                                         
+0/-1     
consolidated_view_test.exs
Remove redundant test module imports                                         
+0/-2     
query_from_test.exs
Remove redundant test module imports                                         
+0/-1     
query_parse_and_build_test.exs
Remove redundant test module imports                                         
+0/-1     
query_result_test.exs
Remove redundant test module imports                                         
+0/-1     
sampling_cache_test.exs
Remove redundant test module imports                                         
+0/-2     
sampling_test.exs
Remove redundant test module imports                                         
+0/-2     
grace_period_test.exs
Remove redundant test module imports                                         
+0/-1     
accept_test.exs
Remove redundant test module imports                                         
+0/-2     
invite_to_site_test.exs
Remove redundant test module imports                                         
+0/-2     
invite_to_team_test.exs
Remove redundant test module imports                                         
+0/-2     
reject_test.exs
Remove redundant test module imports                                         
+0/-2     
remove_from_site_test.exs
Remove redundant test module imports                                         
+0/-1     
layout_test.exs
Remove redundant test module imports                                         
+0/-2     
leave_test.exs
Remove redundant test module imports                                         
+0/-2     
remove_test.exs
Remove redundant test module imports                                         
+0/-2     
update_role_test.exs
Remove redundant test module imports                                         
+0/-2     
transfer_test.exs
Remove redundant test module imports                                         
+0/-2     
teams_test.exs
Remove redundant test module imports                                         
+0/-2     
billing_test.exs
Remove redundant test module imports                                         
+0/-2     
notice_test.exs
Remove redundant test module imports                                         
+0/-1     
flow_progress_test.exs
Remove redundant test module imports                                         
+0/-1     
external_controller_test.exs
Remove redundant test module imports                                         
+0/-1     
external_sites_controller_sites_crud_api_test.exs
Remove redundant test module imports                                         
+0/-2     
external_sites_controller_test.exs
Remove redundant test module imports                                         
+0/-2     
aggregate_test.exs
Remove redundant test module imports                                         
+0/-2     
auth_test.exs
Remove redundant test module imports                                         
+0/-1     
breakdown_test.exs
Remove redundant test module imports                                         
+0/-1     
query_comparisons_test.exs
Remove redundant test module imports                                         
+0/-1     
query_consolidated_view_test.exs
Remove redundant test module imports                                         
+0/-2     
query_test.exs
Remove redundant test module imports                                         
+0/-1     
query_timezone_test.exs
Remove redundant test module imports                                         
+0/-1     
query_validations_test.exs
Remove redundant test module imports                                         
+0/-1     
timeseries_test.exs
Remove redundant test module imports                                         
+0/-1     
segments_controller_test.exs
Remove redundant test module imports                                         
+0/-1     
sync_test.exs
Remove redundant test module imports                                         
+0/-1     
internal_controller_test.exs
Remove redundant test module imports                                         
+0/-1     
paddle_controller_test.exs
Update to use imported random_ip function                               
+1/-1     
Additional files
88 files
authorization_test.exs +0/-1     
conversions_test.exs +0/-1     
custom_prop_breakdown_test.exs +0/-1     
funnels_test.exs +0/-2     
main_graph_test.exs +0/-1     
pages_test.exs +0/-1     
regions_test.exs +0/-1     
sources_test.exs +0/-1     
top_stats_test.exs +0/-1     
auth_controller_test.exs +2/-4     
billing_controller_test.exs +1/-2     
error_report_controller_test.exs +0/-1     
google_analytics_controller_test.exs +0/-1     
help_scout_controller_test.exs +0/-3     
invitation_controller_test.exs +0/-1     
settings_controller_test.exs +1/-3     
membership_controller_test.exs +0/-5     
site_controller_test.exs +0/-4     
sso_controller_sync_test.exs +0/-2     
sso_controller_test.exs +0/-4     
stats_controller_test.exs +0/-2     
unsubscribe_controller_test.exs +0/-1     
email_test.exs +0/-2     
change_domain_test.exs +0/-3     
choose_plan_test.exs +1/-2     
combo_box_test.exs +0/-1     
form_test.exs +0/-1     
verification_test.exs +1/-3     
sites_test.exs +0/-3     
teams_test.exs +0/-3     
users_test.exs +0/-3     
customer_support_test.exs +0/-3     
form_test.exs +0/-1     
funnel_settings_test.exs +0/-3     
form_test.exs +0/-2     
goal_settings_sync_test.exs +0/-1     
goal_settings_test.exs +0/-2     
installation_test.exs +0/-3     
plugins_api_tokens_test.exs +0/-1     
form_test.exs +0/-2     
props_settings_test.exs +0/-2     
register_form_test.exs +0/-3     
reset_password_form_test.exs +0/-1     
form_test.exs +0/-2     
shared_link_settings_test.exs +0/-2     
countries_test.exs +0/-1     
hostnames_test.exs +0/-1     
ip_addresses_test.exs +0/-1     
pages_test.exs +0/-1     
sites_test.exs +0/-2     
sso_management_test.exs +0/-2     
team_management_test.exs +0/-2     
team_setup_test.exs +0/-2     
verification_test.exs +0/-1     
capabilities_test.exs +0/-1     
custom_props_test.exs +0/-1     
funnels_test.exs +0/-2     
goals_test.exs +0/-1     
tracker_script_configuration_test.exs +0/-1     
auth_plug_test.exs +0/-1     
authorize_public_api_test.exs +0/-1     
authorize_site_access_test.exs +0/-1     
authorize_team_access_test.exs +0/-1     
handle_expired_session_test.exs +0/-3     
require_account_plug_test.exs +0/-1     
sso_team_access_test.exs +0/-3     
tracker_plug_test.exs +0/-1     
tracker_script_cache_test.exs +0/-2     
tracker_test.exs +0/-2     
user_auth_test.exs +0/-2     
test_schema.ex +1/-0     
dev_subscriptions.ex +1/-0     
accept_traffic_until_test.exs +0/-1     
check_usage_test.exs +0/-1     
clean_invitations_test.exs +0/-1     
clickhouse_clean_sites_test.exs +0/-2     
import_analytics_test.exs +0/-1     
lock_sites_test.exs +0/-1     
notify_annual_renewal_test.exs +0/-1     
notify_exported_analytics_test.exs +0/-2     
schedule_email_reports_test.exs +1/-1     
send_check_stats_emails_test.exs +0/-1     
send_email_report_test.exs +1/-2     
send_site_setup_emails_test.exs +0/-1     
send_trial_notifications_test.exs +0/-1     
set_legacy_time_on_page_cutoff_test.exs +0/-2     
sso_domain_verification_worker_test.exs +0/-2     
traffic_change_notifier_test.exs +0/-1     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Out-of-scope Change: The PR only refactors test modules and imports without adding or modifying application
logic that performs critical actions, so audit trail compliance cannot be assessed from
this diff.

Referred Code
refute_receive :paddle_queried

new_ip =
  Plug.Conn.put_req_header(initial_conn, "x-forwarded-for", random_ip())

conn = get(new_ip, Routes.paddle_path(initial_conn, :currency))
assert json_response(conn, 200) == %{"currency" => "£"}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Test Refactor Only: Changes are limited to test setup and helper imports (e.g., replacing
Plausible.TestUtils.random_ip/0 with random_ip/0), so robustness of production error
handling cannot be evaluated from this diff.

Referred Code
  })
  |> recycle()
  |> Map.put(:secret_key_base, secret_key_base())
  |> Plug.Conn.put_req_header("x-forwarded-for", random_ip())
end

defp set_remember_2fa_cookie(conn, user) do
  conn
  |> PlausibleWeb.TwoFactor.Session.maybe_set_remember_2fa(user, "true")
  |> recycle()
  |> Map.put(:secret_key_base, secret_key_base())
  |> Plug.Conn.put_req_header("x-forwarded-for", random_ip())
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
No App Changes: The diff only modifies test helpers/imports and does not alter user-facing error messages
or logging behavior, so secure error handling cannot be judged here.

Referred Code
defmodule PlausibleWeb.Api.StatsController.AuthorizationTest do
  use PlausibleWeb.ConnCase

  describe "API authorization - as anonymous user" do

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Logging Not Visible: Only test import and use changes are present; no new or modified logging statements in
application code are introduced to assess secure logging.

Referred Code
use Bamboo.Test

import Phoenix.View

alias PlausibleWeb.Endpoint
alias PlausibleWeb.ErrorView

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Test Scope Only: The PR centralizes test utilities and imports but does not modify input validation or data
handling in production code, so security validation cannot be assessed from this diff.

Referred Code
using do
  quote do
    use Plausible.TestUtils
    use Plausible
    use Plausible.Teams.Test

    import Plausible.Test.Support.HTML
    import Plug.Conn
    import Phoenix.ConnTest

    alias PlausibleWeb.Router.Helpers, as: Routes
    import Plausible.Factory
    import Plausible.AssertMatches

    @endpoint PlausibleWeb.Endpoint
  end
end

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore accidentally removed security test

Restore the accidentally removed test case test "goal names are HTML safe". This
test is important for preventing potential XSS vulnerabilities.

test/plausible_web/controllers/site_controller_test.exs [871-877]

   assert html_response(conn, 200) =~ "Custom event"
   assert html_response(conn, 200) =~ "Visit /register"
 end
 
 test "goal names are HTML safe", %{conn: conn, site: site} do
   insert(:goal, site: site, event_name: "<some_event>")
 
   conn = get(conn, "/#{site.domain}/settings/goals")
 
+  assert html_response(conn, 200) =~ "&lt;some_event&gt;"
+end
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that a security-related test case for HTML escaping was accidentally removed and should be restored to prevent potential XSS vulnerabilities.

High
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants