Implement subpackage option for local e2e test runs

[daledupreez]

This PR explores the addition of a subpackage option to the qit run:e2e command that can be used for local e2e test runs to limit execution to one or more subpackages defined for the current local directory. The option can only be used when exactly one local test package is specified, and will error in all other cases.

The gap/use case for this was flagged in this comment on woocommerce/woocommerce-gateway-stripe#5208 by @zhongruige -- this PR tries to add support in QIT itself.

I am not actually sure how to test this in my environment, and I put this together using Claude, so we may want to drop the feature.

[Copilot]

Pull request overview

Adds a --subpackage option to qit run:e2e to allow limiting local E2E runs to one or more subpackages defined in a single local test package manifest, with supporting manifest-synthesis logic and new unit/integration tests.

Changes:

• Introduces --subpackage on run:e2e, including fail-fast validation and expanding a local parent test package into per-subpackage executions.
• Adds TestPackageManifest::create_subpackage_manifest() and reuses it from the downloader flow.
• Adds new utility helpers/tests (SubpackageSelector, expand_local_path) plus integration coverage for selection and execution.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/src/Commands/RunE2ECommand.php	Adds --subpackage option, validation, and runtime expansion of a local parent package into subpackage entries.
src/src/Commands/Environment/UpEnvironmentCommand.php	Reuses shared local-path expansion helper for --test-package normalization.
src/src/PreCommand/Objects/TestPackageManifest.php	Adds subpackage manifest synthesis (create_subpackage_manifest).
src/src/PreCommand/Download/TestPackageDownloader.php	Refactors subpackage extraction to call create_subpackage_manifest.
src/src/Utils/SubpackageSelector.php	New validator for --subpackage constraints against a single local package manifest.
src/src/Utils/PackageReferenceUtils.php	Adds expand_local_path() helper and tests for path expansion behavior.
src/tests/unit/TestPackageManifestTest.php	Adds unit tests for create_subpackage_manifest behavior and error cases.
src/tests/unit/SubpackageSelectorTest.php	Adds unit tests for SubpackageSelector::validate_selection().
src/tests/unit/PackageReferenceUtilsTest.php	Adds unit tests for expand_local_path().
src/tests/integration/tests/TestPackages/Packages/Subpackages/LocalSubpackageSelectionTest.php	Adds integration tests ensuring --subpackage validation fails fast (no Docker needed).
src/tests/integration/tests/TestPackages/Packages/Subpackages/LocalSubpackageExecutionTest.php	Adds Docker integration tests verifying only selected subpackages run and shared phases deduplicate.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[zhongruige]

Ah! This is great @daledupreez! You beat me to looking into this! I'll go through this asap, just wanted to comment so you know it's on my radar!

[Copilot]

Pull request overview

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

[zhongruige]

I think the chnages here work for the intended use case: selecting a local subpackage replaces the parent package’s run phase while inheriting its setup, teardown, requirements, and result configuration.

For example, this produces the expected package:

qit run:e2e \
  --config=qit.json \
  --profile=main \
  --subpackage=<slug>/<package-name> \
  --env_file=./tests/e2e/qit/qit.au.env

I found one behavior that I think should be fixed in this PR, which I left a comment on.

There's work we should do on top of this (I don't think we need to do it on this PR and what is done here is worth having), so I suggest a follow-up that makes subpackage selection part of common package resolution:

• Resolve local and remote subpackages before env:up.
• Attach selection to its parent package so multiple local parents are unambiguous.
• Support selections in profiles and groups.
• Merge environment requirements and report conflicts.
• Give local and Manager-backed subpackages the same resolved-package representation.

Overall otherwise this is looking good!

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/tests/unit/SubpackageSelectorTest.php:168

• This test calls expectExceptionMessage() multiple times; only the last call is effective, so it won’t actually verify that the available subpackage IDs are listed. Use expectExceptionMessageMatches() to assert the full message contains all expected substrings.

		$this->expectException( \RuntimeException::class );
		$this->expectExceptionMessage( "Subpackage 'checkout' not found in test package 'woocommerce/e2e-suite'" );
		$this->expectExceptionMessage( 'woocommerce/checkout' );
		$this->expectExceptionMessage( 'woocommerce/cart' );

src/tests/integration/tests/Commands/RunE2ECommandTest.php:361

• The test asserts that a subpackage-specific setup phase runs ("Setting up SUBPACKAGE"), but the implementation intentionally inherits all parent phases and only overrides run. This assertion will fail (and conflicts with the unit tests for create_subpackage_manifest).

			if ( $subpackage ) {
				$this->assertStringContainsString( 'Setting up SUBPACKAGE', $output );
			}

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/src/PreCommand/Objects/TestPackageManifest.php:465

• create_subpackage_manifest() builds the subpackage manifest from $this->to_array() (normalized form), but the normalized representation does not include actions, and load_from_normalized() never initializes the typed $actions property. As a result, subpackage manifests won’t inherit parent actions, and calling get_actions() on a synthesized subpackage manifest would trigger a typed-property initialization fatal.

Consider adding actions to to_array() and initializing it in load_from_normalized() (defaulting to [] when absent) so subpackage manifests truly inherit parent configuration safely.

		// Start with parent's complete configuration.
		$package_data = $this->to_array();

		$package_data['package_id']     = $subpackage_id;
		$package_data['parent_package'] = $this->package_id;

[Copilot]

Pull request overview

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

[zhongruige]

A few things I noticed while tracing the local subpackage path through manifest serialization and execution.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/src/Utils/SubpackageSelector.php:33

• Local package directories are deduplicated using raw strings, but expand_local_path() returns absolute paths unchanged. That means the same directory can be counted twice if referenced with different spellings (e.g. /path vs /path/, or via a symlink), causing an incorrect “requires exactly ONE local test package” failure. Canonicalize the directory (realpath + rtrim) before adding to $local_dirs and before the uniqueness check.

				$local_dir = PackageReferenceUtils::expand_local_path( $ref );

src/src/Utils/SubpackageSelector.php:145

• validate_subpackage_id() accepts any string containing a slash that doesn’t start/end with /. This allows IDs like namespace/package/subpackage (multiple slashes) or namespace//x, which then get split with explode('/', 2) elsewhere and can leak / into $package_name. That can produce unsafe container directory names/paths and contradicts the stated required format namespace/subpackage. Tighten validation to exactly one slash, non-empty parts, and no version suffix (:).

	private static function validate_subpackage_id( string $subpackage_id ): void {
		if ( str_contains( $subpackage_id, '/' ) && ! str_starts_with( $subpackage_id, '/' ) && ! str_ends_with( $subpackage_id, '/' ) ) {
			return;
		}

[zhongruige]

@daledupreez just wanted to check if you were ready for me to take another look at this. Thanks!

[daledupreez]

@zhongruige, yes, I would love for you to take another look!

[zhongruige]

Thanks for addressing the earlier review threads. I found four additional edge cases in the current head and left inline comments with suggested fixes and coverage.

[zhongruige]

[P1] Please reject explicitly empty selections here instead of filtering them away. Symfony parses --subpackage= as ['']; this array_filter() turns that into [], skips the entire selection branch, and the normal path runs the full parent package. This is especially easy to hit with --subpackage="$TARGET" when an environment variable is unset, and it contradicts the later “full parent package was NOT run” safety guarantee. Please check raw option presence and fail on any empty or whitespace-only value, with coverage for --subpackage=.

[zhongruige]

[P1] This synthesis happens only after run_environment() has already provisioned the environment using the parent directory. Any allowed subpackages.*.requires override for plugins, themes, network, or tunnel is therefore ignored for a local selection. The new manifest unit fixture even has a cart subpackage requiring woocommerce-gateway-stripe, but that dependency will not be installed unless the parent also requires it. Please derive and merge the selected requirements before env:up, or teach env:up about the selection, so local behavior matches remote subpackage semantics.

[zhongruige]

[P2] Multiple selected siblings each get a metadata entry pointing at this same parent path. PackagePhaseRunner::generate_actions_manifest() scans every metadata entry and reads the raw parent manifest, so the same action provider/path is appended once per selected sibling. qit.actions() then returns duplicate callables and consumers can execute the same action twice. Please deduplicate by provider/action path (or contribute runtime metadata once per physical package) and add coverage for multiple selected subpackages whose parent declares actions.

[zhongruige]

[P2] array_unique() compares path spellings rather than directory identities. expand_local_path() deliberately returns absolute paths unchanged, so /path/package and /path/package/.—or a directory and a symlink alias—are counted as two local parents. I reproduced the error with the fixture path plus /.. Please canonicalize each validated directory with realpath() before deduplicating and counting it.