Add tests for `AbstractServiceProvider` and `ExtendedContainer`.

Also:

- Make the methods in `AbstractServiceProvider` protected.
- Add an autoloader for files in the `tests/php/src` directory.
- Fix a bug in the provisional (?) autoloader.

src/Autoloader.php

@@ -42,7 +42,7 @@ class Autoloader {
 			return false;
 		}
 
-		self::registerPsr4Autoloader();
+		self::register_psr4_autoloader();
 
 		$autoloader_result = require $autoloader;
 		if ( ! $autoloader_result ) {
@@ -59,11 +59,11 @@ class Autoloader {
 	 * TODO: Assess if this is still needed after https://github.com/Automattic/jetpack/pull/15106 is merged.
 	 *       If it still is, remove this notice. If it isn't, remove the method.
 	 */
-	protected static function registerPsr4Autoloader() {
+	protected static function register_psr4_autoloader() {
 		spl_autoload_register(
 			function ( $class ) {
 				foreach ( self::NON_CORE_WOO_NAMESPACES as $non_core_namespace ) {
-					if ( $non_core_namespace === substr( $class, 0, strlen( $non_core_namespace ) ) ) {
+					if ( substr( $class, 0, strlen( $non_core_namespace ) ) === $non_core_namespace ) {
 						return;
 					}
 				}

src/DependencyManagement/AbstractServiceProvider.php

@@ -38,11 +38,11 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 	 *
 	 * @throws \Exception Error when reflecting the class, or class constructor is not public, or an argument has no valid type hint.
 	 */
-	public function add_with_auto_arguments( string $class_name, $concrete = null, bool $shared = false ) : DefinitionInterface {
+	protected function add_with_auto_arguments( string $class_name, $concrete = null, bool $shared = false ) : DefinitionInterface {
 		try {
 			$reflector = new \ReflectionClass( $class_name );
 		} catch ( \ReflectionException $ex ) {
-			throw new \Exception( get_class( $this ) . "::addWithAutoArguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
+			throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
 		}
 
 		$definition = new Definition( $class_name, $concrete );
@@ -51,7 +51,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 
 		if ( ! is_null( $constructor ) ) {
 			if ( ! $constructor->isPublic() ) {
-				throw new \Exception( get_class( $this ) . "::addWithAutoArguments: constructor of class '$class_name' isn't public, instances can't be created." );
+				throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: constructor of class '$class_name' isn't public, instances can't be created." );
 			}
 
 			$constructor_arguments = $constructor->getParameters();
@@ -62,7 +62,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 				} else {
 					$argument_class = $argument->getClass();
 					if ( is_null( $argument_class ) ) {
-						throw new \Exception( get_class( $this ) . "::addWithAutoArguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
+						throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
 					}
 
 					$definition->addArgument( $argument_class->name );
@@ -90,7 +90,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 	 *
 	 * @throws \Exception Error when reflecting the class, or class constructor is not public, or an argument has no valid type hint.
 	 */
-	public function share_with_auto_arguments( string $class_name, $concrete = null ) : DefinitionInterface {
+	protected function share_with_auto_arguments( string $class_name, $concrete = null ) : DefinitionInterface {
 		return $this->add_with_auto_arguments( $class_name, $concrete, true );
 	}
 
@@ -103,7 +103,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 	 *
 	 * @return DefinitionInterface The generated container definition.
 	 */
-	public function add( string $id, $concrete = null, bool $shared = null ) : DefinitionInterface {
+	protected function add( string $id, $concrete = null, bool $shared = null ) : DefinitionInterface {
 		return $this->getContainer()->add( $id, $concrete, $shared );
 	}
 
@@ -115,7 +115,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 	 *
 	 * @return DefinitionInterface The generated container definition.
 	 */
-	public function share( string $id, $concrete = null ) : DefinitionInterface {
+	protected function share( string $id, $concrete = null ) : DefinitionInterface {
 		return $this->add( $id, $concrete, true );
 	}
 }

tests/legacy/bootstrap.php

@@ -67,6 +67,9 @@ class WC_Unit_Tests_Bootstrap {
 		// load WC testing framework.
 		$this->includes();
 
+		// register autoloader for tests in 'src'.
+		$this->register_psr4_autoloader();
+
 		// re-initialize dependency injection, this needs to be the last operation after everything else is in place.
 		$this->initialize_dependency_injection();
 	}
@@ -131,6 +134,26 @@ class WC_Unit_Tests_Bootstrap {
 		$GLOBALS['wc_container'] = $inner_container;
 	}
 
+	/**
+	 * Register autoloader for the files in the 'tests/php/src' directory.
+	 */
+	protected static function register_psr4_autoloader() {
+		spl_autoload_register(
+			function ( $class ) {
+				$prefix   = 'Automattic\\WooCommerce\\Tests\\';
+				$base_dir = __DIR__ . '/php/src/';
+				$len      = strlen( $prefix );
+				if ( strncmp( $prefix, $class, $len ) !== 0 ) {
+					// no, move to the next registered autoloader.
+					return;
+				}
+				$relative_class = substr( $class, $len );
+				$file           = $base_dir . str_replace( '\\', '/', $relative_class ) . '.php';
+				require $file;
+			}
+		);
+	}
+
 	/**
 	 * Load WooCommerce.
 	 *

tests/php/src/DependencyManagement/AbstractServiceProviderTests.php

@@ -0,0 +1,128 @@
+<?php
+/**
+ * AbstractServiceProviderTests class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement;
+
+use Automattic\WooCommerce\DependencyManagement\AbstractServiceProvider;
+use Automattic\WooCommerce\DependencyManagement\ExtendedContainer;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\ClassWithConstructorArgumentWithoutTypeHint;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\ClassWithDependencies;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\ClassWithPrivateConstructor;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\DependencyClass;
+use League\Container\Definition\DefinitionInterface;
+
+/**
+ * Tests for AbstractServiceProvider.
+ */
+class AbstractServiceProviderTests extends \WC_Unit_Test_Case {
+
+	/**
+	 * The system under test.
+	 *
+	 * @var AbstractServiceProvider
+	 */
+	private $sut;
+
+	/**
+	 * The container used for tests.
+	 *
+	 * @var ExtendedContainer
+	 */
+	private $container;
+
+	/**
+	 * Runs before each test.
+	 */
+	public function setUp() {
+		$this->container = new ExtendedContainer();
+
+		$this->sut = new class() extends AbstractServiceProvider {
+			// phpcs:disable
+
+			/**
+			 * Public version of add_with_auto_arguments, which is usually protected.
+			 */
+			public function add_with_auto_arguments( string $class_name, $concrete = null, bool $shared = false ) : DefinitionInterface {
+				return parent::add_with_auto_arguments( $class_name, $concrete, $shared );
+			}
+
+			/**
+			 * The mandatory 'register' method (defined in the base class as abstract).
+			 * Not implemented because this class is tested on its own, not as a service provider actually registered on a container.
+			 */
+			public function register() {}
+
+			// phpcs:enable
+		};
+
+		$this->sut->setContainer( $this->container );
+	}
+
+	/**
+	 * @testdox 'add_with_auto_arguments' should throw an exception if an invalid class name is passed.
+	 *
+	 * @throws \Exception Invalid class name passed.
+	 */
+	public function test_add_with_auto_arguments_throws_on_non_class_passed() {
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: error when reflecting class 'foobar': Class foobar does not exist" );
+
+		$this->sut->add_with_auto_arguments( 'foobar' );
+	}
+
+	/**
+	 * @testdox 'add_with_auto_arguments' should throw an exception if the passed class has a private constructor.
+	 *
+	 * @throws \Exception The passed class has a private constructor.
+	 */
+	public function test_add_with_auto_arguments_throws_on_private_constructor() {
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: constructor of class '" . ClassWithPrivateConstructor::class . "' isn't public, instances can't be created." );
+
+		$this->sut->add_with_auto_arguments( ClassWithPrivateConstructor::class );
+	}
+
+	/**
+	 * @testdox 'add_with_auto_arguments' should throw an exception if the passed class has a constructor argument without type hint.
+	 *
+	 * @throws \Exception The passed class has a constructor argument without type hint.
+	 */
+	public function test_add_with_auto_arguments_throws_on_constructor_argument_without_type_hint() {
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: constructor argument 'argument_without_type_hint' of class '" . ClassWithConstructorArgumentWithoutTypeHint::class . "' doesn't have a type hint or has one that doesn't specify a class." );
+
+		$this->sut->add_with_auto_arguments( ClassWithConstructorArgumentWithoutTypeHint::class );
+	}
+
+	/**
+	 * @testdox 'add_with_auto_arguments' should properly register the supplied class.
+	 *
+	 * @testWith [true, 1]
+	 *           [false, 2]
+	 *
+	 * @param bool $shared Whether to register the test class as shared or not.
+	 * @param int  $expected_constructions_count Expected number of times that the test class will have been instantiated.
+	 */
+	public function test_add_with_auto_arguments_works_as_expected( bool $shared, int $expected_constructions_count ) {
+		ClassWithDependencies::$instances_count = 0;
+
+		$this->container->share( DependencyClass::class );
+		$this->sut->add_with_auto_arguments( ClassWithDependencies::class, null, $shared );
+
+		$this->container->get( ClassWithDependencies::class );
+		$resolved = $this->container->get( ClassWithDependencies::class );
+
+		// A new instance is created for each resolution or not, depending on $shared.
+		$this->assertEquals( $expected_constructions_count, ClassWithDependencies::$instances_count );
+
+		// Arguments with default values are honored.
+		$this->assertEquals( ClassWithDependencies::SOME_NUMBER, $resolved->some_number );
+
+		// Constructor arguments are filled as expected.
+		$this->assertSame( $this->container->get( DependencyClass::class ), $resolved->dependency_class );
+	}
+}

tests/php/src/DependencyManagement/ExampleClasses/ClassWithConstructorArgumentWithoutTypeHint.php

@@ -0,0 +1,22 @@
+<?php
+/**
+ * ClassWithConstructorArgumentWithoutTypeHint class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses;
+
+/**
+ * An example class that has a constructor argument without type hint.
+ */
+class ClassWithConstructorArgumentWithoutTypeHint {
+
+	/**
+	 * Class constructor.
+	 *
+	 * @param mixed $argument_without_type_hint Anything, really.
+	 */
+	public function __construct( $argument_without_type_hint ) {
+	}
+}

tests/php/src/DependencyManagement/ExampleClasses/ClassWithDependencies.php

@@ -0,0 +1,52 @@
+<?php
+/**
+ * ClassWithDependencies class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses;
+
+/**
+ * An example of a class with dependencies that are supplied via constructor arguments.
+ */
+class ClassWithDependencies {
+
+	/**
+	 * Default value for $some_number argument.
+	 */
+	const SOME_NUMBER = 34;
+
+	/**
+	 * Count of instances of the class created so far.
+	 *
+	 * @var int
+	 */
+	public static $instances_count = 0;
+
+	/**
+	 * Value supplied to constructor in $some_number argument.
+	 *
+	 * @var int
+	 */
+	public $some_number = 0;
+
+	/**
+	 * Value supplied to constructor in $dependency_class argument.
+	 *
+	 * @var DependencyClass
+	 */
+	public $dependency_class = null;
+
+	/**
+	 * Class constructor.
+	 *
+	 * @param DependencyClass $dependency_class A class we depend on.
+	 * @param int             $some_number Some number we need for some reason.
+	 */
+	public function __construct( DependencyClass $dependency_class, int $some_number = self::SOME_NUMBER ) {
+		self::$instances_count++;
+		$this->dependency_class = $dependency_class;
+		$this->some_number      = $some_number;
+	}
+}

tests/php/src/DependencyManagement/ExampleClasses/ClassWithPrivateConstructor.php

@@ -0,0 +1,20 @@
+<?php
+/**
+ * ClassWithPrivateConstructor class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses;
+
+/**
+ * An example of a class with a private constructor.
+ */
+class ClassWithPrivateConstructor {
+
+	/**
+	 * Class constructor.
+	 */
+	private function __construct() {
+	}
+}

tests/php/src/DependencyManagement/ExampleClasses/DependencyClass.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * DependencyClass class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses;
+
+/**
+ * An example of a class other classes depend on.
+ */
+class DependencyClass {
+}

tests/php/src/DependencyManagement/ExtendedContainerTests.php

@@ -0,0 +1,104 @@
+<?php
+/**
+ * ExtendedContainerTests class file.
+ *
+ * @package Automattic\WooCommerce\Tests\DependencyManagement
+ */
+
+namespace Automattic\WooCommerce\Tests\DependencyManagement;
+
+use Automattic\WooCommerce\DependencyManagement\ExtendedContainer;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\ClassWithDependencies;
+use Automattic\WooCommerce\Tests\DependencyManagement\ExampleClasses\DependencyClass;
+
+/**
+ * Tests for ExtendedContainer.
+ */
+class ExtendedContainerTests extends \WC_Unit_Test_Case {
+
+	/**
+	 * The system under test.
+	 *
+	 * @var ExtendedContainer
+	 */
+	private $sut;
+
+	/**
+	 * Runs before each test.
+	 */
+	public function setUp() {
+		$this->sut = new ExtendedContainer();
+	}
+
+	/**
+	 * @testdox 'add' should throw an exception when trying to register a class not in the WooCommerce root namespace.
+	 *
+	 * @throws \Exception Attempt to register a class not in the WooCommerce root namespace.
+	 */
+	public function test_add_throws_when_trying_to_register_class_in_forbidden_namespace() {
+		$external_class = \League\Container\Container::class;
+
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "Can't use the container to register '" . $external_class . "', only objects in the Automattic\WooCommerce namespace are allowed for registration." );
+
+		$this->sut->add( $external_class );
+	}
+
+	/**
+	 * @testdox 'add' should allow registering classes in the WooCommerce root namespace.
+	 */
+	public function test_add_allows_registering_classes_in_woocommerce_root_namespace() {
+		$instance = new DependencyClass();
+		$this->sut->add( DependencyClass::class, $instance, true );
+		$resolved = $this->sut->get( DependencyClass::class );
+
+		$this->assertSame( $instance, $resolved );
+	}
+
+	/**
+	 * @testdox 'replace' should throw an exception when trying to replace a class that has not been previously registered.
+	 *
+	 * @throws \Exception Attempt to replace a class that has not been previously registered.
+	 */
+	public function test_replace_throws_if_class_has_not_been_registered() {
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "ExtendedContainer::replace: The container doesn't have '" . DependencyClass::class . "' registered, please use 'add' instead of 'replace'." );
+
+		$this->sut->replace( DependencyClass::class, null );
+	}
+
+	/**
+	 * @testdox 'replace' should allow to replace existing registrations.
+	 */
+	public function test_replace_allows_replacing_existing_registrations() {
+		$instance_1 = new DependencyClass();
+		$instance_2 = new DependencyClass();
+
+		$this->sut->add( DependencyClass::class, $instance_1, true );
+		$this->assertSame( $instance_1, $this->sut->get( DependencyClass::class ) );
+
+		$this->sut->replace( DependencyClass::class, $instance_2, true );
+		$this->assertSame( $instance_2, $this->sut->get( DependencyClass::class ) );
+	}
+
+	/**
+	 * @testdox 'reset_resolved' should discard cached resolutions for classes registered as 'shared'.
+	 */
+	public function test_reset_resolved_discards_cached_shared_resolutions() {
+		$this->sut->add( DependencyClass::class );
+		$this->sut->add( ClassWithDependencies::class, null, true )->addArgument( DependencyClass::class );
+		ClassWithDependencies::$instances_count = 0;
+
+		$this->sut->get( ClassWithDependencies::class );
+		$this->assertEquals( 1, ClassWithDependencies::$instances_count );
+		$this->sut->get( ClassWithDependencies::class );
+		$this->assertEquals( 1, ClassWithDependencies::$instances_count );
+
+		$this->sut->reset_resolved();
+
+		$this->sut->get( ClassWithDependencies::class );
+		$this->assertEquals( 2, ClassWithDependencies::$instances_count );
+		$this->sut->get( ClassWithDependencies::class );
+		$this->assertEquals( 2, ClassWithDependencies::$instances_count );
+	}
+}