Add host PHPUnit unit suite; refactor Query and Connectors for testability (XWPENG-42)

[shadyvb]

Fixes XWPENG-42.

Introduces a fast, host-native PHPUnit unit tier (composer test-unit) using Brain Monkey, then refactors Connectors and Query into testable, named SQL-fragment methods. Along the way, several latent bugs in Query::where_in(), pagination, meta ordering, and filter-arg handling are fixed and locked in with data-provider tests.

Summary

Host unit suite

• New phpunit-unit.xml, tests/phpunit/unit/bootstrap.php (Composer autoload + WP_Stream\ class autoload mirroring Plugin::autoload).
• New composer test-unit script; dev dependency yoast/wp-test-utils (Brain Monkey).
• Integration configs (phpunit.xml, phpunit-multisite.xml) exclude tests/phpunit/unit/ so Docker/integration runs stay unchanged.
• contributing.md documents composer test-unit (host) and Docker wrapper npm run test:php-unit.
• npm run test runs test:php-unit as a separate step before test:php; unit tests are not merged into the integration PHPUnit config.

Connectors refactor

• Connectors::BUILTIN_CONNECTOR_SLUGS constant replaces inline slug list.
• Memoized load pipeline: get_available_connectors() → instantiate_connector_classes() → register_connector_instances().
• get_slugs( $include_inactive = false ) and get_all( $include_inactive = false ) replace get_all_including_admin_only() / get_all_slugs_including_admin_only().
• Abilities updated: exclusion-rule validation uses get_slugs( true ); get-connectors uses get_all( true ) behind new filter wp_stream_abilities_connectors.
• Connector: get_label(), get_context_labels(), get_action_labels() promoted to abstract (all 22 built-in connectors already implement them).

Query refactor

• Monolithic query() split into public fragment methods: where_columns(), where_dates(), where_in(), select(), orderby(), join(), limits().
• New ORDERABLE_FIELDS constant documents intentional sort behavior.
• Public $wpdb property set in constructor (enables partial mocks in unit tests).
• Meta sort: join() emits LEFT JOIN streammeta when orderby is meta_value / meta_value_num and meta_key is set.
• Meta JOIN deduplication: GROUP BY stream.ID on the items query; count query uses COUNT( DISTINCT stream.ID ).

Bugs fixed in original Query code

Bug	Before	After
where_in single-element arrays	array_shift() removed the only value, then empty( $value ) skipped the clause entirely	All values kept via array_values(); one-element IN (1) works
record__in field mapping	str_replace( 'record_', … ) left _in suffix → invalid column	Suffix strip + explicit record → ID mapping
where_in prepare() with array arg	$wpdb->prepare( …, $field, $value ) passed the whole array as one scalar → empty/wrong SQL	Spread operator ...$values with per-value %d/%s placeholders
__not_in family	Separate loop with same array_shift / prepare bugs	Unified suffix-based handler for both __in and __not_in
paged = 0 pagination	absint(0) → offset (0-1)*per_page wrapped to large positive offset (page 2 behavior)	$page < 1 clamped to 1 → LIMIT 0, per_page
Meta orderby without JOIN	ORDER BY streammeta.meta_value with no JOIN (broken SQL)	join() adds LEFT JOIN streammeta when meta sort + meta_key
Meta JOIN duplicate rows	No dedup on multi-meta rows	GROUP BY stream.ID + COUNT( DISTINCT stream.ID )
Search clause order	Regressed during refactor (search after action)	Restored: search sits between user_role and connector
Filter $args date leak	Passing date mutated caller args with date_from/date_to before filters ran	where_dates() expands on a local copy only; filter hooks receive original args

Not restored (intentional): bare __in / __not_in args no longer map to ID; use record__in instead.

Intentional behavior preserved

• orderby = date and orderby = ip fall back to sorting by ID.
• records_per_page = 0 still emits LIMIT 0, 0.
• Connectors with unsatisfied dependencies remain in instances but are not registered.

Breaking changes

Change	Risk
Abstract label methods on Connector	Low — all built-ins implement them; GitHub search found no third-party gaps
Old get_all_including_admin_only() methods removed	Use get_all( true ) / get_slugs( true )
Filter $args no longer receive date_from/date_to when only date passed	Intentional fix of pre-refactor leak

Testing

Command: composer test-unit → OK (53 tests, 106 assertions)

File	Coverage highlights
test-connectors.php	Skip gates, happy path, load/filter, get_slugs/get_all, unload/reload
test-query.php	All SQL fragments, filter-args immutability, meta JOIN assembly

Accepted gaps: Connectors constructor/include loop; Query __construct, empty search_field fallback, invalid IP, date boundaries.

Gotchas

• PHPCS ignores on allowlisted column interpolation in loops.
• Meta JOIN + GROUP BY only when meta sort active.
• composer test-unit is host fast path; Docker uses npm run test:php-unit.
• contributing.md references npm run test-unit but package.json script is test:php-unit.

Commits

1. a5dbc77 — Host unit suite + Connectors pipeline + connector tests
2. c3c62d4 — Query refactor + unit tests + bug fixes

Checklist

• Project documentation has been updated to reflect the changes in this pull request, if applicable.
• I have tested the changes in the local development environment (see contributing.md).
• I have added phpunit tests.

Release Changelog

• Fix: Query::where_in() silently dropped single-value filters, mis-mapped record__in, and produced invalid SQL with array prepare args.
• Fix: Query pagination treated paged = 0 as large positive offset.
• Fix: Meta-value sorting referenced streammeta without JOIN; duplicate rows deduplicated via GROUP BY / COUNT(DISTINCT).
• Fix: date arg expansion no longer mutates filter-hook $args.
• New: Host-native PHPUnit unit suite (composer test-unit).
• New: Connectors::get_slugs() / get_all() with $include_inactive.
• New: wp_stream_abilities_connectors filter.

Release Checklist

• This pull request is to the master branch.
• Release version follows semantic versioning. Does it include breaking changes?
• Update changelog in readme.txt.
• Bump version in stream.php.
• Bump Stable tag in readme.txt.
• Bump version in classes/class-plugin.php.
• Draft a release on GitHub.

[Copilot]

Pull request overview

Adds a host-native PHPUnit suite and refactors connector loading and query construction for testability.

Changes:

• Adds Brain Monkey unit testing and supporting commands/configuration.
• Refactors connector discovery and metadata APIs.
• Splits query construction into testable SQL-fragment methods.

Critical issues

• Meta sorting generates invalid SQL because GROUP BY follows ORDER BY. Swap the clauses. WP VIP ref: valid $wpdb query construction.

Medium/Low issues

• Documentation references a nonexistent npm script; use test:php-unit.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
classes/class-query.php	Refactors query fragments and meta sorting.
classes/class-connectors.php	Refactors connector loading and accessors.
classes/class-connector.php	Makes label methods abstract.
abilities/class-ability-get-connectors.php	Uses the new connector metadata API.
abilities/class-ability-create-exclusion-rule.php	Uses the new slug API.
tests/phpunit/unit/test-query.php	Tests query fragments and assembly.
tests/phpunit/unit/test-connectors.php	Tests connector lifecycle behavior.
tests/phpunit/unit/bootstrap.php	Bootstraps host unit tests.
phpunit-unit.xml	Configures the unit suite.
phpunit.xml	Excludes unit tests from integration runs.
phpunit-multisite.xml	Excludes unit tests from multisite runs.
composer.json	Adds test dependencies and command.
composer.lock	Locks new testing dependencies.
package.json	Adds the Docker unit-test wrapper.
contributing.md	Documents unit-test commands.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bartoszgadomski]

@shadyvb Thank you for working on this PR! Please check AI-assisted inline comments below.

[bartoszgadomski]

Behaviour change: abilities now expose connectors whose dependencies aren't satisfied.

get_all( true ) and get_slugs( true ) read $connector_instances, which holds every instantiated builtin plus filter extras, with no is_dependency_satisfied() gate. The removed build_full_connector_classes() did skip unsatisfied connectors.

So on a site where WooCommerce / Jetpack / EDD aren't active, stream/get-connectors now lists those connectors and stream/create-exclusion-rule accepts their slugs — Connector_Woocommerce::is_dependency_satisfied() returns false without WC, and previously that removed it from both ability payloads.

test_get_slugs_and_get_all (include_inactive case) locks the new behaviour in, so this looks deliberate rather than accidental. If it is, the ability description should say the list is "connectors this plugin ships" rather than "what exists on this site"; if not, apply the dependency gate in the $include_inactive branch the way the old helper did.

[bartoszgadomski]

Meta sort picks an arbitrary value from the group.

With GROUP BY stream.ID and ORDER BY streammeta.meta_value, the sort key is an arbitrary row from each group, so records with more than one row for the same meta_key sort on the wrong value. stream_meta has no unique key on (record_id, meta_key), so duplicates are possible.

Reproduced on MySQL 8.4 with this exact generated SQL: record 1 has meta values 5 and 9, record 2 has 7. Result order was 2, 1, 3 — record 1 sorted on 5 instead of 9, so it landed below record 2.

ORDER BY MAX( streammeta.meta_value ) (and MAX( CAST( ... AS SIGNED ) ) for meta_value_num) fixes the ordering and, as a bonus, makes the statement valid under ONLY_FULL_GROUP_BY. As written it's ERROR 1055 under that mode; it only runs because wpdb::set_sql_mode() strips ONLY_FULL_GROUP_BY from its session, which is re-addable via the incompatible_sql_modes filter.

Worth noting the coverage gap too: test_query_meta_orderby_includes_join only string-matches the JOIN and ORDER BY; nothing asserts the GROUP BY or runs the SQL against a real DB. This path isn't reachable from the admin UI or the abilities layer today (meta_key isn't in DB::get_records() defaults and meta_value isn't in the get-records orderby enum), so it would break on the first caller that passes both args.

[bartoszgadomski]

This drops two fail-open guards: isset( $this->plugin->connectors ) and ! empty( $known ).

Plugin::$connectors is declared without a default and only assigned in the init callback, so a direct execute() before that point now fatals on a method call on null instead of skipping validation. And if the slug list ever comes back empty — e.g. a wp_stream_connectors filter returning array() — every connector is now rejected rather than validation being bypassed.

Both are edge cases, but the empty-list check in particular seems worth keeping unless failing closed is the intent.

[bartoszgadomski]

Nit: $class_names is ignored whenever $connector_instances is already an array, so the parameter is only honoured on the first call — an easy trap for a later caller passing a different list and getting the memoized one back.

Related: load_connectors() assigns the post-filter set back to $connector_instances, so a second load_connectors() call re-applies wp_stream_connectors to an already-filtered list and permanently drops any builtin a filter removed. Only the constructor calls it today, so this is latent. Reading the memo inside the method instead of taking it as a parameter would make the contract clearer.

[bartoszgadomski]

Nit: unset( $unused ) is a no-op on a by-value parameter, so this line can go. Keeping the parameter itself makes sense for BC — DB_Driver_WPDB still calls new Query( $this ) — but naming it $unused and discarding it hides that a driver is still being passed in. Either keep the DB_Driver $driver name (documented as unused) or store it.

[bartoszgadomski]

Nit: plugin-api-version goes 2.9.0 → 2.6.0, meaning this lock was regenerated with an older Composer than the one that produced the committed file. Unrelated churn in a dependency PR — re-running the lock update on the project's Composer version would keep this diff limited to the Brain Monkey / Mockery / wp-test-utils additions.