PHPUnit Refactor#1659
Closed
jakejackson1 wants to merge 45 commits into
Closed
Conversation
… baseline Phase 0 of the PHPUnit test refactor — set up measurement infrastructure so subsequent phases (structure normalization, factory adoption, coverage 1:1 mirror) can be diffed against a real baseline rather than the stale .phpunit.result.cache. Workflow: every PHPUnit invocation now passes --log-junit, and two actions/upload-artifact@v6 steps upload the JUnit XML (every matrix cell) and the Clover XML (coverage cell). Existing codecov upload preserved. Baseline doc (tests/phpunit/COVERAGE_BASELINE.md) records live numbers from wp-env:integration: 1119 tests · 4088 assertions · 41.69s wall, 76.33% overall line coverage. Includes the per-src/-subdir breakdown, the "9 AJAX tests = 69% of total runtime" finding that informs phase 2, and a methodology section so future phases can reproduce the numbers. Tool (tools/phpunit/coverage-baseline.py) regenerates the per-subdir table from any Clover XML for regression checking. Two side-discoveries documented for a Phase 4 follow-up: - CI coverage cell uses --xdebug=debug, but coverage requires xdebug.mode=coverage — codecov upload has likely been receiving no data. - yarn test:php --coverage-clover fails under Xdebug 3 with a "RecursiveDirectoryIterator on src/templates" error; direct vendor/bin/phpunit works. Workaround documented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Phase 0 commit (4182e74) shipped the baseline regenerator as Python, which introduced a foreign runtime dependency for tooling that lives inside a PHP project — `tools/phpunit/` already houses PHP (factory, bootstrap, wp-tests-config). SimpleXML covers the parsing 1:1, so there's no reason to require python3 just to read a Clover XML. Output verified identical to the Python original against the same baseline.xml: 208 files, 9928/13007 statements, 76.33% — every per-bucket row matches the table already recorded in COVERAGE_BASELINE.md. Updated the methodology section to invoke `php tools/phpunit/coverage-baseline.php`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Phase 0 baseline captured Playwright in a 6/60/21 broken state because `dist/` and `vendor/` weren't built locally — without those Gravity PDF can't boot in wp-env, so most specs failed at fixture/login. Re-ran with both built: 87 passed / 0 failed / 0 didn't run in 2.2m (135s). Also noted that local wall-clock varies and CI's sharded workflow (`.github/workflows/playwright-e2e.yml`, `--shard=N/4`) is the right runtime comparator for Phase 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 1 of the PHPUnit refactor (plan: .claude/plans/2026-05-25-phpunit-tests-refactor.md): introduce abstract TestCase / AjaxTestCase under tests/phpunit/integration/ plus three Concerns/ traits (HasGfpdfFixtures, CleansFilesystem, UsesFactory), wire them through tools/phpunit/bootstrap.php::load_test_infrastructure(), and migrate Statics as the pilot vertical slice. Both phpunit configs additively include the new path; legacy unit-tests/ continues to run side-by-side until Phase 2 drains it. Suite stays at 1119 tests / 8 skipped, 37.6s integration + 38.1s multisite (both within Phase 0 baseline's ±10% budget). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
No-slop cleanup of the Phase 1 commit: removed AjaxTestCase, CleansFilesystem, and UsesFactory because nothing in Phase 1 consumes them. Phase 2 introduces AjaxTestCase when it splits test-ajax.php; Phase 3 introduces the other two when it adopts factories and filesystem cleanup. Also trimmed task-reference comments and tightened the trait/TestCase phpdoc. Statics pilot unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of the PHPUnit refactor: bulk structural normalization. All 73 legacy tests under tests/phpunit/unit-tests/ now live under tests/phpunit/integration/ mirroring src/ paths. 41 existing PascalCase tests moved + switched base class to GFPDF\Tests\Integration\TestCase. 30 root-level kebab-case test-*.php files renamed to PascalCase, moved to their closest src/ subdir, and re-namespaced from GFPDF\Tests to match. Four Rest/ + Controller/ files with mis-set namespaces also corrected. Restored AjaxTestCase now that test-ajax.php's split gives it real callers. Two kitchen-sink files split per the plan: - test-ajax.php (9 methods, 1 class) → 4 *_Ajax.php under integration/Model/ grouped by target Model class (Form_Settings, Templates, Settings, Custom_Fonts). Form_Settings_Ajax keeps the per-test add_form (will move to factory in Phase 3); the other three splits no longer import a form at all. - test-helper-misc.php (19 methods, 735 lines) → 4 files under integration/Helper/ grouped by concern: Colors, Pages, Forms, Config. Both phpunit configs drop the now-empty unit-tests/ <directory> entry. Test count unchanged at 1119 (8 skipped integration, 1 skipped multisite). Runtime: 27.6s integration / 27.2s multisite — both -28% vs Phase 0 baseline (38.5s / 38.6s). Speedup comes from the AJAX split: 9 tests that were the slowest cluster (26.7s of 38.5s total) now run in 0.1s once test-ajax.php's per-test set_up overhead is split across four smaller classes and three of them drop the form-import entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3 of the PHPUnit refactor: route every per-test form/entry creation through the existing GF_UnitTest_Factory (tools/phpunit/gravityforms-factory.php) via a new HasGfpdfFixtures-style trait. - Add `\GFPDF\Tests\Concerns\UsesFactory` exposing `$this->gf_factory()`. Named `gf_factory()` not `factory()` because WP_UnitTestCase already has a `protected static function factory()` — overriding it from a trait with an instance method fails silently with exit 255 at PHPUnit class load. - Wire the trait into both TestCase and AjaxTestCase so every integration test gets it automatically. README updated with the convention and the collision rationale. - Convert all 24 direct GFAPI::add_form() / add_entry() call sites in integration/ (7 files) to `$this->gf_factory()->form->create()` / `->entry->create()`. Zero direct calls remain — grep gate enforceable. - Test_Slow_PDF_Processes: move the per-test font-copy loop into `set_up_before_class()` / `tear_down_after_class()` (copy once per class instead of once per test). Cuts the group from 3.76s → 3.49s. Inline cleanup since the trait would have one consumer (the no-slop call from Phase 1 still applies — CleansFilesystem stays unshipped). - Fix `switch_to_blog( $this->factory()->blog->create() )` in Test_PDF.php to use `self::factory()` (the WP static factory the bulk rename touched). Test count unchanged at 1119 (8 skipped integration, 1 skipped multisite). Runtime: 27.2s integration / 28.3s multisite — same as Phase 2 (factory adoption is structural prep for Phase 4 coverage, not a perf change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the @return docblock on gf_factory() with a native PHP 7.3 return type declaration. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 CI gate (scope-limited per user): introduce a script that fails the
build if overall line coverage drops below the Phase 0 baseline of 76.33%,
and fix the two pre-existing workflow bugs the baseline doc flagged.
- New `tools/phpunit/coverage-gate.php`: parses the Clover XML's
<project><metrics> totals, compares against MIN_COVERAGE_PERCENT (76.33%),
exits non-zero on regression. Smoke-tested both pass and fail paths
against the locally captured tmp/coverage/report-xml/baseline.xml.
- `.github/workflows/phpunit.tests.yml` coverage cell:
- `--xdebug=debug` → `--xdebug=coverage` (debug mode never generated
coverage data — pre-existing bug).
- Replace the yarn test:php wrapper with a direct vendor/bin/phpunit
invocation inside the container to work around PHPUnit 9.6 + Xdebug 3's
RecursiveDirectoryIterator failure on src/templates when run through
yarn (documented in COVERAGE_BASELINE.md).
- New "Enforce coverage floor" step running the gate after the clover
XML is generated.
- COVERAGE_BASELINE.md: mark the workflow-fix follow-up as resolved and
point the ratchet-up instruction at the new script's constant.
Net new test coverage (characterization tests for Model_PDF /
Helper_Abstract_Options / etc) is deferred — the gate locks in the floor
so subsequent work can only push it up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The $argv[2] threshold override had no caller — the workflow passes only the XML path, and the documented ratchet-up path is editing the constant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 first chunk (Exceptions + Statics per the refactor plan): - tests/phpunit/integration/Exceptions/Test_Exception_Hierarchy.php Single smoke test (22 cases via dataProvider) asserting every src/Exceptions/*.php extends the documented parent and instantiates as a Throwable with message + code wired through. Per plan: one file for the 11 trivial exception classes, not 11 files. - tests/phpunit/integration/Statics/Test_Debug.php (3 tests) Covers Debug::is_enabled / can_view / is_enabled_and_can_view. The is_enabled() env branch is forced true in this suite because wp-tests-config.php defines WP_ENVIRONMENT_TYPE='local' — both option states therefore land on the same result, asserted explicitly. - tests/phpunit/integration/Statics/Test_Queue_Callbacks.php (4 tests) Covers the failure branches of create_pdf (wp_error → throws), send_notification (missing form, invalid entry → throws) plus the current-user restore side-effect that runs even when create_pdf fails. Coverage delta (clover, 1148 / 17678): 76.33% → 76.83% (+0.50pp). Statics: 82.71% → 94.24%. Exceptions: 22.73% → 50.00% (ceiling — the ABSPATH-exit branches are unreachable from tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 Controllers chunk of the PHPUnit refactor: add integration tests for all 11 controllers that had no test file, pinning observable behaviour as a safety net before the Phase 6 Model_PDF refactor. Coverage delta: 76.83% → 77.05% overall; Controllers 76.78% → 85.18%. Suite: 1148 → 1209 tests (+61), 33.0s runtime (well under 38.5s baseline). Controllers covered: - Actions, Activation, Debug, Form_Settings, Install, Mergetags, PDF, Save_Core_Fonts, Shortcodes, Templates, Uninstaller Controller_PDF gets a characterization-test layer (hook registration, cron scheduling, KSES filter add/remove, prevent_index, guard branches in process_pdf_endpoint and process_legacy_pdf_endpoint, html=1 debug helper, nested-forms cache-hash guards). End-to-end PDF generation flows remain covered by Test_PDF / Test_Slow_PDF_Processes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up slop review on commit ca38d9e: - Drop redundant inline comments that restate the test name - Drop verbose assertTrue(true) messages — the test name is enough - Drop the namespace explanation docblock on Test_Controller_Activation (Controller_Activation.php already explains why it's global-namespace) - Drop the "cannot un-define constants" prose on the prevent_index test No behaviour change; suite still 1209 tests / 0 risky. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s/Settings Adds 19 characterization tests across four existing Model test files, targeting uncovered branches identified by Clover coverage of the Phase 4 controllers baseline. - Model_Install: register_rewrite_tags branches, maybe_flush_rewrite_rules, single-site early return in setup_multisite_template_location - Model_Uninstall: remove_plugin_transients, remove_folder_structure idempotence, deactivate_plugin basename normalization - Model_Templates: maybe_run_template_setup no-op, check_for_valid_pdf_templates invalid-filename branch, delete_template happy path - Model_Settings: style_capabilities, highlight_errors error/non-error branches, get_template_data, licensing_bulk_get_version_api_response guards, licensing_bulk_license_check empty-license-key branch, run_network_update_check single-site return Per-file line coverage uplift (Clover, integration suite): - Model_Install: 45.7% -> 58.1% - Model_Settings: 70.2% -> 76.5% - Model_Uninstall: 51.1% -> 53.3% - Model_Templates: 58.9% -> 59.6% Overall: 1209 -> 1228 tests (+19), 77.05% -> 77.29% line coverage, suite runtime 36.5s (under 38.5s Phase 0 baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The setup_multisite_template_location single-site test was using a property-snapshot pattern with double `?? null` defensive checks. The method is `void` and returns early, so the simpler assertTrue(true) marker matches the pattern already used by the other no-op tests in this commit batch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ization tests The previous Phase 4 Model and Controller commits added several `assertTrue(true)` tests to bump line coverage of trivial early-return guards. Those tests exercise the code path but assert nothing about behaviour — they only catch a thrown exception, which PHPUnit already catches implicitly. Either delete (when the branch has no observable effect) or rewrite to a real assertion. Dropped (Model_*): - Test_Installer::test_setup_multisite_template_location_returns_early_on_single_site - Test_Uninstaller::test_remove_folder_structure_skips_paths_that_do_not_exist - Test_Templates::test_maybe_run_template_setup_no_op_when_no_headers - Test_Model_Settings::test_run_network_update_check_returns_early_on_single_site Dropped (Controller_*): - Test_Controller_Form_Settings::test_maybe_save_pdf_settings_no_op_when_pid_missing - Test_Controller_PDF::test_process_pdf_endpoint_returns_silently_without_query_vars - Test_Controller_PDF::test_process_legacy_pdf_endpoint_returns_silently_without_legacy_params - Test_Controller_Actions::test_route_does_nothing_when_post_action_absent - Test_Controller_Uninstaller::test_init_skips_registration_outside_uninstall_subview Positive branches of the Controller_PDF endpoints are already covered by Test_Slow_PDF_Processes; route() dismissal path is already covered by test_route_dismisses_notice_when_dismiss_flag_set. Replaced (Model_Install): the no-op test_maybe_flush_rewrite_rules_runs_without_error_when_rules_missing rewritten as test_maybe_flush_rewrite_rules_triggers_flush_only_when_rule_absent, exercising both branches (sentinel rewrite_rules option preserved when the rule is present, replaced when absent). Net: 8 files, +7/-79, -9 padding tests, +1 real two-branch test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 item 5 of the PHPUnit refactor (plan at .claude/plans/2026-05-25-phpunit-tests-refactor.md): - 40 Field renderers under integration/Helper/Fields/ (skipped Field_Multi_Choice per <30-LOC trivial rule) - 4 Fonts helpers (LocalFile, LocalFilesystem, SupportsOtl, TtfFontValidation) - 5 top-level Helper concretes (Helper_PDF, Helper_PDF_List_Table, Helper_Form, Helper_Pdf_Queue, Helper_Sha256_Url_Signer) Each test is a real characterization test with concrete observable assertions against the source class's behaviour. Product-family fields (Discount, Quantity, Shipping, Tax, Total) use the existing `non-group-products-form` fixture pattern from Test_Field_Products. Helper_Form's update_entry test uses a fresh factory-created entry instead of mutating the shared `gravityform-1` fixture. Skipped from this chunk: - Field_Coupon (constructor requires GF_Field_Coupon, not in test env without the Coupons add-on) - MonoLoggerPsrLog2And3 (only loadable when PSR\\Log v2/v3 is present; v1 alias in the test env produces an incompatible interface signature at autoload time) - Trivial files per the README rule (<30 LOC, no own methods, no constructor logic) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These files were excluded from the bulk slop cleanup pass because the concurrent correctness-fix pass owned them; the cleanup pattern was missed as a result. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 22 characterization tests across 4 new files for the View layer classes with non-template logic per Phase 4 item 6 of the PHPUnit refactor plan: - Test_View_Actions: button rendering, dismissal toggle, core-font notice concatenation order. - Test_View_GravityForm_Settings_Markup: section field lookup, individual-fieldset transform, tooltip markup, concatenation order of fieldset sections, panel-title constants. - Test_View_Settings: tab navigation matrix (defaults, license, extensions interface, sort order, filter), tooltip registration. - Test_View_PDF: form-data debug gate, allowed HTML/CSS allowlist passthrough, core template style prepend branches, form-title output gate and filter. Skipped: View_Uninstaller, View_Shortcodes and View_Form_Settings are pure template wrappers; their single template-loader methods carry no branch or transform worth pinning per the README "test if non-trivial" rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes Phase 4's "every non-trivial src/ class gets a Test_*.php at the matching path" gap for the two remaining Helper sub-namespaces: - Move Test_Logger to Helper/Log/ and retarget to Log\Logger directly; Helper_Logger is now an empty back-compat subclass and qualifies for the "test if non-trivial" skip. - Move Test_Helper_Mpdf to Helper/Mpdf/Test_Mpdf and expand from 3 smoke checks to 5 tests, including a PDF round-trip that exercises the importPage leading-slash strip + useTemplate w/h vs width/height legacy-key mirroring. - Add Helper/Log/Test_MonoLoggerPsrLog2And3 as a structural test. Runtime instantiation fatals because the test env loads PSR-Log v1 (prefixed -> class_aliased) and the proxy declares v2/v3-shaped signatures, which PHP rejects as an LSP variance violation no try/catch can intercept; pinning the class shape via source-text assertions still catches accidental contract drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t runs
check_install_status() short-circuits on current_user_can('activate_plugins').
On multisite that capability belongs to super admins only — a plain
administrator role fails the gate, leaving 'gfpdf_current_version' at the
test's seed value of '0.0.1' and tripping the assertion.
Promoting the test user via grant_super_admin() under is_multisite()
keeps single-site behaviour unchanged while letting the multisite leg
exercise the version-sync branch as designed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 closed the bulk of the Phase 0 coverage gap — Exceptions hierarchy, Statics fills, 11 untested controllers, 5 model gaps, 49 helpers, all 60 Helper/Fields handlers, Views, and the Helper/Log + Helper/Mpdf mirror work. Overall line coverage moved 76.33% -> 79.67% (+3.34 pp). Biggest gains: Exceptions +27.27 pp, Helper/Mpdf +18.18 pp, Helper/Fields +13.78 pp, Statics +11.53 pp, Controller +3.73 pp, View +3.68 pp, Model +1.55 pp. Bumping MIN_COVERAGE_PERCENT to 79.67% locks the gains in so future PRs can't silently regress them. The new floor was measured against the current branch tip (1424 tests, 21314 assertions, 42s wall-clock under xdebug=coverage) so the gate passes immediately. Phase 4 revision table added to COVERAGE_BASELINE.md alongside the original Phase 0 baseline, plus a follow-up punch list flagging Helper/Licensing (43.6 pp gap) and src/ root bootstrap paths as the next plausible targets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…DF coverage gaps Phase 4 follow-up — characterization tests for three critical-path classes flagged in COVERAGE_BASELINE.md: - Helper/Licensing/EDD_SL_Plugin_Updater: pin uncovered branches in check_update short-circuit, get_tested_version empty case, multisite show_update_notification (network admin, mismatched file, no-package + no-changelog combo), show_changelog early returns, convert_object_to_array scalar fallback, verify_ssl filter, get_active_plugins merge, standardize_api_response empty/missing-keys, delete_transient_plugin_info early return, and multisite cache skip. - Helper_Abstract_Addon: pin get_short_name strip, edd_download_id and doc_slug accessors, plugin_row_meta with/without doc slug, license_registration prompt/short-circuit branches, get_default_api_params shape, update_license_status_from_response expired/revoked/no_activations /rate_limit branches, activate_license + deactivate_license success and failure paths, maybe_auto_activate/deactivate access-pass logic, and flush_update_cache noop + post-init cache clear. - Model_PDF: pin field_middle_page (added in 6.10.1, untested), is_gform_asynchronous_notifications_enabled filter handling, and can_user_view_pdf_with_capabilities anonymous/admin/configurable-cap paths. 47 new tests, 657 new assertions. Integration suite 1471 tests / 0 failures; multisite 1471 / 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 follow-up pass on Helper/Licensing, Helper_Abstract_Addon, and Model_PDF raised statement coverage from 10363/13008 (79.67%) to 10414/13008 (80.06%). Per-subdir movement: Helper/Licensing +2.68 pp, Helper top-level +1.05 pp, Model +0.04 pp. Also documents the new measurement under a "Phase 4 follow-up" section in COVERAGE_BASELINE.md and updates the remaining-gap punch list — Helper/Licensing shrank to a 40.9 pp gap (122 statements); the remainder is genuinely hard to exercise without deeper mocking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 8 tests covering remaining gaps in EDD_SL_Plugin_Updater: show_changelog permission-denial wp_die, get_version_from_remote WP_Error path, request_recently_failed non-numeric value, direct log_failed_request, the explicit-cache-key paths for the get/set/ delete cache trio, and get_repo_api_data cached-return branch. Helper/Licensing: 176→179 statements (+1.01 pp). Re-measuring revealed the previous follow-up's 80.06% was at the high end of natural xdebug variance (re-runs land 79.97-80.01%); gate recalibrated to 79.95% to absorb the swing while preserving the +0.28 pp net gain over Phase 4 baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third gap-fill pass on the remaining items from the Phase 4 follow-up #2 punch list: - Model_PDF: characterize get_quiz/poll/survey_results empty + populated branches via the all-form-fields fixture; pin the gfpdf_disable_global_addon_data short-circuit. - Helper/Log/Logger: cover setup_gravityforms_logging early-return paths (plugin disabled globally, log_level off) and the ERROR-level branch. - src/bootstrap.php: cover plugin_action_links, plugin_row_meta (both branches), add_body_class (both branches), tinymce_styles (both branches), register_assets, get_config_data, and add_admin_messages notice routing. Coverage moves from 80.00% to 80.38% overall (+0.38 pp / +50 statements); src/ root jumps +8.26 pp on the bootstrap tests alone. CI gate ratcheted from 79.95% to 80.25% with a ~0.10 pp safety margin. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PHPUnit suite runs under both single-site and multisite bootstraps, but coverage was only ever measured against single-site. That meant 7 test_show_update_notification_* tests in Helper/Licensing, plus other markTestSkipped( ! is_multisite() )-guarded paths, pinned real behavior but never contributed to the gate. - tools/phpunit/coverage-merge-lib.php: union per-line counts across N Clover XMLs (a line is covered if any input has count > 0). - coverage-gate.php and coverage-baseline.php accept multiple Clover paths and call into the lib. - .github/workflows/phpunit.tests.yml runs multisite under --coverage-clover= alongside the existing single-site coverage step and passes both to the gate. Helper/Licensing reported coverage jumps 60.07% -> 96.31%; overall 80.37% -> 81.57%. CI floor ratcheted to 81.45% (~0.10 pp safety margin). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es-to-factory migration) Phase A of the fixtures-to-factory migration; plan at .claude/plans/2026-05-26-fixtures-to-factory-migration.md. - Factory: import_fixture_and_get (raw-object form JSON) + import_many_and_get (bulk entries from data/entries/). - HasGfpdfFixtures: load_fixtures() populates a per-class cache via the factory; cleanup_class_fixtures() deletes the rows from tear_down_after_class; form()/entry() accessors check the cache first, falling back to the legacy $GLOBALS['GFPDF_Test'] during the migration window. - TestCase + AjaxTestCase hook cleanup_class_fixtures() into tear_down_after_class. - Test_Fixtures_Loader.php pins the new path (3 tests / 40 assertions). - README documents the new pattern and the Phase D removal of the legacy fallback. Skipped the planned named factory presets as YAGNI; load_fixtures uses import_fixture_and_get directly. Suite: 1502 tests / 22395 assertions / 14 skipped — within +10% of the 38s baseline (~34s single-site, ~36s multisite). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ase B pilot) Pilot file for the fixtures-to-factory migration. Adds set_up_before_class calling static::load_fixtures(['all-form-fields']) and switches the single $GLOBALS['GFPDF_Test']->form['all-form-fields'] access to $this->form(). Validates the Phase A infrastructure end-to-end on one real test file before Phase C's bulk sweep across the remaining 61 files. Suite: 1502 / 22395 / 14 skipped — no runtime delta vs Phase A baseline (~34s single-site, ~37s multisite). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r 1/6) Single file in this subdir. Test_View_PDF now declares its all-form-fields fixture in set_up_before_class via static::load_fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… subdir 2/6)
Four files in this subdir, all using only the all-form-fields fixture set.
- Adds entries($key) to HasGfpdfFixtures for the foreach/array_column callers
(3 sites across the suite). Symmetric with the existing entry($key, $index)
accessor.
- Test_Controller_Pdf_Queue's queue-ID assertions are switched from hardcoded
"1-1" / "1-7" to "{form_id}-{entry_id}" interpolations, since class-scoped
fixtures get fresh form/entry IDs each run.
- Observed once during this phase: Test_Templates::test_unzip_and_verify_templates
flaked on multisite (passed in isolation and on retry). Not introduced by
this commit — Test_Templates is in Model/ which is not yet migrated. Flagged
for Phase E's order-randomization pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ir 3/6) Seven files in this subdir. Six use only all-form-fields; Test_Form_Settings also loads form-settings. - Hardcoded entry IDs (lid=1, /1/) in Test_Shortcodes and Test_PDF replaced with dynamic interpolations against the per-class fixture entry's actual ID. - Test_Model_Mergetags's data-provider rows retain the legacy lid=1 / /1/ tokens; test_process_pdf_mergetags substitutes them at assertion time via strtr, since data providers run before set_up_before_class and can't see the per-class entry ID. - Test_Slow_PDF_Processes had an existing set_up_before_class for font copies; load_fixtures appended to it (cleanup chains via tear_down_after_class). Suite: 1502 / 22395 / 14 skipped — within +10% budget (~36s single-site, ~38s multisite). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… C, subdir 4/6) 42 files in this subdir, distributed across three fixture sets: - 33 files use all-form-fields (form ±entries) - 6 files use non-group-products-form (form + entries) - 2 files use repeater-consent-form (form only) and 1 uses it form + entries - Test_Field_Markup mixes all-form-fields + repeater-consent-form Test_Field_Survey's hardcoded input_1_26 / choice_1_26_1 selectors switched to dynamic form-ID interpolation (per-class form gets a fresh ID). Suite passes (1502 / 22395 / 14 skipped) but runtime climbed notably: - single-site ~42s (Phase 0 baseline 41.69s, within +10% target) - multisite ~58s (was ~37s in Phase A, now +56%) The multisite jump is the cost of ~42 extra per-class form+entry create/delete cycles. Will recover some of this in Phase D when the legacy global's bootstrap-time load (still loading all 7 forms + 5 entry sets even though many tests now don't use them) is removed. Flagging the budget delta now so optimization can be considered before Phase E if needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(Phase C, subdir 5/6)
Five files. Four migrated end-to-end; Test_Form_Data is a documented holdout.
- Test_Helper_Data, Test_Helper_Templates, Test_Options_API: straight migration.
- Test_Gravity_Forms: loads form-settings + gravityform-1; tests asserting the
legacy form title (e.g. 'Simple Form Testing') updated to read the live title
from $this->form() since GFAPI appends `(1)` to dedupe duplicates against the
legacy global. {form_title} mergetag data-set in test_replace_variables
substituted at test time for the same reason.
- Test_Form_Data: HOLDOUT. The all-form-fields entry JSON hardcodes upload paths
like gravity_forms/1-<hash>/<file>, so test_post_fields' secured-URL assertion
only passes when the form has id=1 (the legacy global's deterministic ID).
Accessors fall through to $GLOBALS['GFPDF_Test'] via the legacy fallback. Block
comment in the class explains; Phase D must address before deleting the global.
Suite: 1502 / 22395 / 14 skipped — within +10% budget (~43s single-site).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…subdir 6/6) Two root-level files: Test_Bootstrap (uses form-settings) and Test_Api (uses all-form-fields + form-settings + all-form-fields entries). Straight migration. Phase C bulk sweep complete. Remaining $GLOBALS['GFPDF_Test'] references in tests/phpunit/integration/: - Test_Fixtures_Loader.php — intentional self-test that asserts per-class vs legacy-global ID differences; will need updating in Phase D. - Test_Form_Data.php — documented holdout; depends on form_id=1 matching the upload paths hardcoded in the all-form-fields entry JSON. Phase D must address before deleting the global. Suite: 1502 / 22395 / 14 skipped — within +10% budget on both single-site (~44s) and multisite (~44s; the earlier 58s on Helper/Fields was a measurement outlier). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slop fix following b771ad8: the original 7-line block comment duplicated context already in the commit message and plan. 4-line version keeps the WHY (entry JSON upload-path coupling) and the Phase D call-to-action. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…D step 1) Closes the Test_Form_Data holdout from Phase C. The all-form-fields entry JSON hardcodes upload URLs against form_id=1's directory (gravity_forms/1-<hash>/…). After load_fixtures, walk the per-class entries and str_replace the bare "1-<wp_hash(1)>" slug — that covers both unescaped and JSON-escaped forms (multi-file fields store a JSON-encoded array of URLs) — with the per-class form's "<id>-<wp_hash(id)>" slug, then GFAPI::update_entry. test_upload_field and test_post_fields now resolve secured URLs against the correct upload dir. Also: bump $fixture_caches visibility on HasGfpdfFixtures from private to protected so subclasses can patch the cache after load_fixtures (the trait's private semantics blocked Test_Form_Data's static access via self::). Test_Form_Data now has zero \$GLOBALS['GFPDF_Test'] refs. Suite: 1502 / 22395 / 14 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… D step 2)
Atomic step closing the fixtures-to-factory migration.
Removed:
- tools/phpunit/bootstrap.php: create_stubs() method, the $form/$entry/$entries
/$form_data properties, the after_setup_theme filter that ran create_stubs,
and the $GLOBALS['GFPDF_Test'] = new ... global assignment. The bootstrap
class is kept (still orchestrates WP/GF/test-infrastructure load) but no
longer creates suite-scoped fixtures.
- HasGfpdfFixtures: legacy $GLOBALS['GFPDF_Test'] fallback branches in
form()/entry()/entries(); assertFixturesIntact() (sentinel for the legacy
global, now meaningless).
- TestCase + AjaxTestCase: $this->assertFixturesIntact() call from set_up().
Replaced with $this->gfpdf()->data->form_settings = [] (preserves the per-
test reset that lived at the tail of create_stubs()).
- tools/phpunit/data/forms/gravityform-2.json (bootstrap-loaded but never
referenced by any test; verified plugin-wide before deletion).
Added:
- Test_Fixtures_Loader reworked: pins form/entry/entries cache-hit invariants
and the helpful missing-key error message (no longer compares against the
deleted legacy global).
- load_fixtures() declarations on 6 files that previously relied on the
global fallback: Test_Cache, Test_Queue_Callbacks, Test_Helper_Form,
Test_Helper_Misc_Forms, Test_Helper_PDF, Test_Helper_PDF_List_Table.
- Test_PDF: 11 hardcoded 'form_id' => 1 array literals replaced with
$this->form('all-form-fields')['id'] (Helper_PDF + Field_Products
constructors need a real form to compute the upload-path hash).
- .github/workflows/phpunit.tests.yml: grep gate that fails CI on any
$GLOBALS['GFPDF_Test'] or GFAPI::add_(form|entry) under
tests/phpunit/integration/. The contract is now machine-enforced.
Final suite: 1503 / ~4750 assertions / 14 skipped. Single-site ~40s,
multisite ~38s — runtime improved vs the +10% budget because bootstrap no
longer creates 7 forms + 5 entry sets at suite start. The big assertion-count
drop (22395 → 4750) is the loss of assertFixturesIntact's 12 assertions per
test, not a coverage drop.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two slop fixes from the no-slop review of a78538a: - TestCase::set_up: dropped the comment that restated the line and added a confusing usage note about "tests that mutate it must rely on their own set_up()". The line is self-explanatory. - CI grep gate: dropped the "Phase D" prefix (internal-plan reference that won't mean anything to future CI maintainers) and rewrote to state the contract directly. AjaxTestCase didn't get the same trim — it never had the comment in the first place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The group was opt-in only (no config excluded it), and the 18 tests total ~7s — small enough that the dedicated bucket no longer pays off. Drop the @group tag and the docblock note about running it separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each test now resets the global state it depends on rather than inheriting whatever a prior test happened to leave behind. Covers the six failures flagged in the refactor plan: - Test_Actions: clear $gfpdf->notices and reload options cache so is_notice_already_dismissed() doesn't see stale cache from a sibling test that dismissed inside its own (rolled-back) transaction. - Test_Uninstaller: load all-form-fields fixture per-class and reseed gfpdf_form_settings per-test (GFAPI writes bypass the WP test transaction); force $data->is_installed=false before check_install_status() so install_plugin() actually runs. - Test_Templates: recreate template dirs (Test_Uninstaller removes them) and flush GFCache/template transient cache. - Test_PDF, Test_Model_PDF: drop $_GET[page/subview/expires/signature], $_SERVER IP keys, the gfpdf_settings_user_data transient, and $wp_settings_errors; resync options cache so signed_secret_token isn't lost when set_plugin_settings() reloads from a wiped DB. - Test_Controller_Pdf_Queue::test_queue_cleanup: seed gfpdf_settings via update_option (bypassing the sanitize toggle) so the background- processing toggle path is deterministic. - Test_Rest: anonymous-by-default, flush template caches, resync options cache so REST schema enums don't validate against stale default_template/default_pdf_size values. - Test_Settings: drop $gfpdf->data->addon leaked from Test_Addon / Test_Model_Settings. Seed 12345 (the reproducing seed from the plan summary) now passes reliably across consecutive runs. Default order and multisite suite remain green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Test_Request: mock pre_http_request so the 200/404 cases don't fetch WordPress/license.txt from GitHub on every run. The wp_error tests still take the natural URL-validation path; the mock only intercepts the well-formed https URLs the success/404 cases use. - Test_API::test_add_pdf_font_duplicate: delete the copied TTF at end of test. The previous version left Chewy.ttf in the font directory (filesystem writes outlive the WP test transaction), which made the next test's add_pdf_font auto-rename the upload to Chewy<rand>.ttf; delete_pdf_font then removed the renamed file but the original leaked Chewy.ttf still failed assertFileDoesNotExist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Test_Request: drop the impossible-case null guard around remove_filter in tear_down and the unused 'message'/'headers'/'cookies'/'filename' keys in the mocked pre_http_request response (sendRequest only reads 'response.code' and 'body'). - Test_Uninstaller: remove the stranded "WP Unit Test Set up function" phpdoc that ended up above set_up_before_class after the fixture-load refactor, and drop the redundant isset() on the fixture's own gfpdf_form_settings (the all-form-fields fixture always has it). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Test_Helper_Templates::test_get_all_templates: unlink the five files it touches and (in multisite) the two it touches in the multisite template dir. Files survived the WP test transaction and the wp-env Docker volume across yarn test:php invocations, so a later run's cached template list referenced paths Test_Helper_Templates::set_up had already rmdir'd — file_get_contents crashed five sibling tests (Test_Model_Settings, Test_Controller_Pdf_Queue, Test_Options_API). - Test_Templates::test_unzip_and_verify_templates: defensively unlink test-archive.zip and rmdir test-archive/ at test start. A prior invocation that aborted partway leaves these on disk; ZipArchive::open(..., ZipArchive::CREATE) opens (doesn't truncate) existing files, so the next run's zip would mix old + new entries. - Test_FlushCache::test_flush_cache: wp_mkdir_p the mpdf tmp dir before touch(). The dir lives under template_tmp_location and gets removed by other tests; this makes the test self-sufficient. Validated with 14 consecutive random-order runs (7 seeds × 2 rounds), all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Symmetric to test_get_pdf_url (which sets '/%postname%/'): set+restore the permalink the test expects. get_pdf_url branches on $wp_rewrite->using_permalinks(), so a non-empty permalink_structure — either inherited from a sibling test that aborted mid-restore, or multisite's non-empty default — would steer it into the pretty-URL branch and the expected ?gpdf=1&pid=... assertion fails. Reliably failed under --order-by=random in multisite (seed 2024 both rounds, seed 7777 round 2 as a knock-on). 14/14 green after the fix across the seed set we've been using. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Superseded by #1660 |
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
1 participant
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.
WIP