Address code review feedback

WordPress · felixarntz · Jul 8, 2022 · Jun 22, 2022 · Jun 22, 2022 · Jun 22, 2022
commit a156b1aa74293fcfeae71a0c905d890b72c1046f
diff --git a/load.php b/load.php
@@ -142,33 +142,27 @@ function( $module_settings ) {
  *
  * @since n.e.x.t
  *
- * @param array $modules List of active module slugs.
- * @return array List of active valid module slugs.
+ * @param string $module Slug of the module.
+ * @return null|string Module slug name on success, null on failure.
  */
-function perflab_get_valid_modules( array $modules = array() ) {
+function perflab_is_valid_module( $module ) {
 
-	if ( ! is_array( $modules ) ) {
-		return array();
+	if ( empty( $module ) ) {
+		return;
 	}
 
-	$valid_modules = array();
-	foreach ( $modules as $module ) {
-
-		// Do not load module if no longer exists.
-		$module_file = plugin_dir_path( __FILE__ ) . 'modules/' . $module . '/load.php';
-		if ( ! file_exists( $module_file ) ) {
-			continue;
-		}
-
-		// Do not load module if it cannot be loaded, e.g. if it was already merged and is available in WordPress core.
-		if ( ! perflab_can_load_module( $module ) ) {
-			continue;
-		}
+	// Do not load module if no longer exists.
+	$module_file = plugin_dir_path( __FILE__ ) . 'modules/' . $module . '/load.php';
+	if ( ! file_exists( $module_file ) ) {
+		return;
+	}
 
-		$valid_modules[] = $module;
+	// Do not load module if it cannot be loaded, e.g. if it was already merged and is available in WordPress core.
+	if ( ! perflab_can_load_module( $module ) ) {
+		return;
 	}
 
-	return $valid_modules;
+	return $module;
 }
 
 /**
@@ -179,7 +173,7 @@ function perflab_get_valid_modules( array $modules = array() ) {
  * @since 1.1.0
  */
 function perflab_get_generator_content() {
-	$active_and_valid_modules = perflab_get_valid_modules( perflab_get_active_modules() );
+	$active_and_valid_modules = array_filter( perflab_get_active_modules(), 'perflab_is_valid_module' );
 
 	return sprintf(
 		'Performance Lab %1$s; modules: %2$s',
@@ -237,8 +231,7 @@ function perflab_can_load_module( $module ) {
  * @since n.e.x.t Renamed to perflab_load_active_and_valid_modules().
  */
 function perflab_load_active_and_valid_modules() {
-	$active_and_valid_modules = perflab_get_valid_modules( perflab_get_active_modules() );
-
+	$active_and_valid_modules = array_filter( perflab_get_active_modules(), 'perflab_is_valid_module' );;
 	if ( empty( $active_and_valid_modules ) ) {
 		return;
 	}

diff --git a/tests/load-tests.php b/tests/load-tests.php
@@ -131,8 +131,9 @@ function() use ( $active_modules ) {
 				return $active_modules;
 			}
 		);
-		$expected = 'Performance Lab ' . PERFLAB_VERSION . '; modules: ' . implode( ', ', $active_modules );
-		$content  = perflab_get_generator_content();
+		$active_modules = array_filter( perflab_get_active_modules(), 'perflab_is_valid_module' );
+		$expected       = 'Performance Lab ' . PERFLAB_VERSION . '; modules: ' . implode( ', ', $active_modules );
+		$content        = perflab_get_generator_content();
 		$this->assertSame( $expected, $content );
 	}
 
@@ -150,50 +151,40 @@ public function test_perflab_render_generator() {
 		$this->assertContains( $expected, $output );
 	}
 
-	public function test_perflab_get_valid_modules() {
-		global $wp_version;
-
-		// Assert that it returns the empty array for dummy modules.
-		$dummy_active_modules = array(
-			'../tests/testdata/demo-modules/javascript/demo-module-1',
-			'../tests/testdata/demo-modules/something/demo-module-2',
-			'../tests/testdata/demo-modules/images/demo-module-3',
-		);
-
-		$output = perflab_get_valid_modules( $dummy_active_modules );
-		$this->assertIsArray( $output );
-		$this->assertSame( array( '../tests/testdata/demo-modules/something/demo-module-2', '../tests/testdata/demo-modules/images/demo-module-3' ), $output );
-
-		// Assert that it returns the array for modules.
-		if ( $wp_version >= '6.1' ) {
-			$active_modules = require plugin_dir_path( PERFLAB_MAIN_FILE ) . 'default-enabled-modules.php';
-			add_filter(
-				'perflab_active_modules',
-				function() use ( $active_modules ) {
-					return $active_modules;
-				}
-			);
-			$output = perflab_get_valid_modules( perflab_get_active_modules() );
+	public function test_empty_module_for_perflab_is_valid_module() {
+		// Assert that it return null for empty module.
+		$this->assertEmpty( perflab_is_valid_module( '' ) );
+	}
 
-			// Remove Image module as it will marge in 6.1.
-			$remove_marge_modules = array_shift( $active_modules );
-			$this->assertSame( $active_modules, $output );
-		}
+	/**
+	 * @dataProvider provider_dummy_valid_modules
+	 */
+	public function test_perflab_is_valid_module( $dummy_module ) {
+		$output = perflab_is_valid_module( $dummy_module );
+		$this->assertNotEmpty( $output );
+		$this->assertIsString( $output );
 	}
 
-	public function test_perflab_can_load_module() {
-		// Assert that it validates the ability to load modules that are not in the core.
-		$demo_modules = array(
-			'javascript/demo-module-1' => false,
-			'something/demo-module-2'  => true,
-			'images/demo-module-3'     => true,
+	public function provider_dummy_valid_modules() {
+		return array(
+			array( '../tests/testdata/demo-modules/something/demo-module-2' ),
+			array( '../tests/testdata/demo-modules/images/demo-module-3' ),
 		);
+	}
 
-		foreach ( $demo_modules as $module => $can_load ) {
-			$output = perflab_can_load_module( '../tests/testdata/demo-modules/' . $module );
-			$this->assertSame( $can_load, $output );
-		}
+	/**
+	 * @dataProvider provider_dummy_can_load_modules
+	 */
+	public function test_perflab_can_load_module( $dummy_can_load_modules, $module_status ) {
+		$this->assertSame( $module_status, perflab_can_load_module( $dummy_can_load_modules ) );
+	}
 
+	public function provider_dummy_can_load_modules() {
+		return array(
+			array( '../tests/testdata/demo-modules/javascript/demo-module-1', false ),
+			array( '../tests/testdata/demo-modules/something/demo-module-2', true ),
+			array( '../tests/testdata/demo-modules/images/demo-module-3', true ),
+		);
 	}
 
 	private function get_expected_default_option() {