From 1008835488447207853c99c99a16faf9fe346991 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 20 Apr 2021 09:42:07 +0200 Subject: [PATCH 1/4] Fix code sniffer violations in stock functions and its tests --- includes/wc-stock-functions.php | 2 +- tests/php/includes/wc-stock-functions-tests.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/includes/wc-stock-functions.php b/includes/wc-stock-functions.php index 70b268836d5..09027c5e7bd 100644 --- a/includes/wc-stock-functions.php +++ b/includes/wc-stock-functions.php @@ -403,7 +403,7 @@ function wc_get_low_stock_amount( WC_Product $product ) { $low_stock_amount = $product->get_low_stock_amount(); if ( '' === $low_stock_amount && $product->is_type( 'variation' ) ) { - $product = wc_get_product( $product->get_parent_id() ); + $product = wc_get_product( $product->get_parent_id() ); $low_stock_amount = $product->get_low_stock_amount(); } diff --git a/tests/php/includes/wc-stock-functions-tests.php b/tests/php/includes/wc-stock-functions-tests.php index 368eab017cf..ce2b747c9c2 100644 --- a/tests/php/includes/wc-stock-functions-tests.php +++ b/tests/php/includes/wc-stock-functions-tests.php @@ -228,8 +228,8 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $product = WC_Helper_Product::create_simple_product( true, array( - 'manage_stock' => true, - 'stock_quantity' => 10, + 'manage_stock' => true, + 'stock_quantity' => 10, ) ); @@ -254,7 +254,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { // Set the variation low stock amount. $variations = $variable_product->get_available_variations( 'objects' ); - $var1 = $variations[0]; + $var1 = $variations[0]; $var1->set_manage_stock( true ); $var1->set_low_stock_amount( $variation_low_stock_amount ); $var1->save(); @@ -292,7 +292,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { // Set the variation low stock amount. $variations = $variable_product->get_available_variations( 'objects' ); - $var1 = $variations[0]; + $var1 = $variations[0]; $var1->set_manage_stock( true ); $var1->set_low_stock_amount( $variation_low_stock_amount ); $var1->save(); @@ -319,7 +319,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { // Don't set the variation low stock amount. $variations = $variable_product->get_available_variations( 'objects' ); - $var1 = $variations[0]; + $var1 = $variations[0]; $this->assertEquals( $parent_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); } @@ -340,7 +340,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { // Don't set the variation low stock amount. $variations = $variable_product->get_available_variations( 'objects' ); - $var1 = $variations[0]; + $var1 = $variations[0]; $var1->set_manage_stock( false ); $this->assertEquals( $site_wide_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); From 76a613a5bb47afda75bf925e17861ec32d9fdb16 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 20 Apr 2021 10:17:50 +0200 Subject: [PATCH 2/4] Modify wc_get_low_stock_amount so that it always returns an integer. Previously, if the product didn't have an explicit low stock value amount the value of the woocommerce_notify_low_stock_amount option, which is a string, was returned verbatim. Also, update related unit tests to create the option value as a string, and to check that the value returned by woocommerce_notify_low_stock_amount is always an integer. --- includes/wc-stock-functions.php | 2 +- .../framework/class-wc-unit-test-case.php | 11 ++++++ .../php/includes/wc-stock-functions-tests.php | 39 ++++++++++++------- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/includes/wc-stock-functions.php b/includes/wc-stock-functions.php index 09027c5e7bd..8e158aa9e4d 100644 --- a/includes/wc-stock-functions.php +++ b/includes/wc-stock-functions.php @@ -411,5 +411,5 @@ function wc_get_low_stock_amount( WC_Product $product ) { $low_stock_amount = get_option( 'woocommerce_notify_low_stock_amount', 2 ); } - return $low_stock_amount; + return intval( $low_stock_amount ); } diff --git a/tests/legacy/framework/class-wc-unit-test-case.php b/tests/legacy/framework/class-wc-unit-test-case.php index 74289f2b413..5426c2ca6ae 100644 --- a/tests/legacy/framework/class-wc-unit-test-case.php +++ b/tests/legacy/framework/class-wc-unit-test-case.php @@ -246,4 +246,15 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase { public function register_legacy_proxy_class_mocks( array $mocks ) { wc_get_container()->get( LegacyProxy::class )->register_class_mocks( $mocks ); } + + /** + * Check if a value is of integer type. + * This function is already built-in in PHPUnit 8 so this one can be removed once we upgrade to that version of newer. + * + * @param mixed $value The value to check. + * @return bool mixed True if the value is of integer type, false otherwise. + */ + protected function assertIsInt( $value ) { + return $this->assertInternalType( 'int', $value ); + } } diff --git a/tests/php/includes/wc-stock-functions-tests.php b/tests/php/includes/wc-stock-functions-tests.php index ce2b747c9c2..74470a86cbc 100644 --- a/tests/php/includes/wc-stock-functions-tests.php +++ b/tests/php/includes/wc-stock-functions-tests.php @@ -192,6 +192,17 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { } } + /** + * Assert that a value is equal to another one and is of integer type. + * + * @param mixed $expected The value $actual must be equal to. + * @param mixed $actual The value to check for equality to $expected and for type. + */ + private function assertIsIntAndEquals( $expected, $actual ) { + $this->assertEquals( $expected, $actual ); + $this->assertIsInt( $actual ); + } + /** * Test wc_get_low_stock_amount with a simple product which has low stock amount set. */ @@ -200,7 +211,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $site_wide_low_stock_amount = 3; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Simple product, set low stock amount. $product = WC_Helper_Product::create_simple_product( @@ -212,7 +223,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { ) ); - $this->assertEquals( $product_low_stock_amount, wc_get_low_stock_amount( $product ) ); + $this->assertIsIntAndEquals( $product_low_stock_amount, wc_get_low_stock_amount( $product ) ); } /** @@ -222,7 +233,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $site_wide_low_stock_amount = 3; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Simple product, don't set low stock amount. $product = WC_Helper_Product::create_simple_product( @@ -233,7 +244,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { ) ); - $this->assertEquals( $site_wide_low_stock_amount, wc_get_low_stock_amount( $product ) ); + $this->assertIsIntAndEquals( $site_wide_low_stock_amount, wc_get_low_stock_amount( $product ) ); } /** @@ -245,7 +256,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $variation_low_stock_amount = 7; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Parent low stock amount NOT set. $variable_product = WC_Helper_Product::create_variation_product(); @@ -259,17 +270,17 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $var1->set_low_stock_amount( $variation_low_stock_amount ); $var1->save(); - $this->assertEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); // Even after turning on manage stock on the parent, but with no value. $variable_product->set_manage_stock( true ); $variable_product->save(); - $this->assertEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); // Ans also after turning the manage stock off again on the parent. $variable_product->set_manage_stock( false ); $variable_product->save(); - $this->assertEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); } /** @@ -282,7 +293,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $variation_low_stock_amount = 7; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Set the parent low stock amount. $variable_product = WC_Helper_Product::create_variation_product(); @@ -297,7 +308,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $var1->set_low_stock_amount( $variation_low_stock_amount ); $var1->save(); - $this->assertEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $variation_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); } /** @@ -309,7 +320,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $parent_low_stock_amount = 5; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Set the parent low stock amount. $variable_product = WC_Helper_Product::create_variation_product(); @@ -321,7 +332,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $variations = $variable_product->get_available_variations( 'objects' ); $var1 = $variations[0]; - $this->assertEquals( $parent_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $parent_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); } /** @@ -332,7 +343,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $site_wide_low_stock_amount = 3; // Set the store-wide default. - update_option( 'woocommerce_notify_low_stock_amount', $site_wide_low_stock_amount ); + update_option( 'woocommerce_notify_low_stock_amount', strval( $site_wide_low_stock_amount ) ); // Set the parent low stock amount. $variable_product = WC_Helper_Product::create_variation_product(); @@ -343,7 +354,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { $var1 = $variations[0]; $var1->set_manage_stock( false ); - $this->assertEquals( $site_wide_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); + $this->assertIsIntAndEquals( $site_wide_low_stock_amount, wc_get_low_stock_amount( $var1 ) ); } } From d214eab1575fed9c48e42a1d9ce1e3ce2f1f18ab Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 20 Apr 2021 16:35:42 +0200 Subject: [PATCH 3/4] Use int cast instead of intval in wc_get_low_stock_amount --- includes/wc-stock-functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/wc-stock-functions.php b/includes/wc-stock-functions.php index 8e158aa9e4d..f8fc3d557ff 100644 --- a/includes/wc-stock-functions.php +++ b/includes/wc-stock-functions.php @@ -411,5 +411,5 @@ function wc_get_low_stock_amount( WC_Product $product ) { $low_stock_amount = get_option( 'woocommerce_notify_low_stock_amount', 2 ); } - return intval( $low_stock_amount ); + return (int) $low_stock_amount; } From b88eb4c9812f7340d2bd385de77a37447a7d3299 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 20 Apr 2021 17:16:59 +0200 Subject: [PATCH 4/4] Rename "assertIsInt" to "assertIsInteger" and make it static - Renaming to prevent conflicts with the existing method in the newer PHPUnit used in PHP 8. - Making it static because "assertIsInt" is static too, so it'll be easier to replace in the future. --- tests/legacy/framework/class-wc-unit-test-case.php | 12 +++++++----- tests/php/includes/wc-stock-functions-tests.php | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/legacy/framework/class-wc-unit-test-case.php b/tests/legacy/framework/class-wc-unit-test-case.php index 5426c2ca6ae..c3255481fef 100644 --- a/tests/legacy/framework/class-wc-unit-test-case.php +++ b/tests/legacy/framework/class-wc-unit-test-case.php @@ -7,6 +7,7 @@ use Automattic\WooCommerce\Proxies\LegacyProxy; use Automattic\WooCommerce\Testing\Tools\CodeHacking\CodeHacker; +use PHPUnit\Framework\Constraint\IsType; /** * WC Unit Test Case. @@ -248,13 +249,14 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase { } /** - * Check if a value is of integer type. - * This function is already built-in in PHPUnit 8 so this one can be removed once we upgrade to that version of newer. + * Asserts that a variable is of type int. + * TODO: After upgrading to PHPUnit 8 or newer, remove this method and replace calls with PHPUnit's built-in 'assertIsInt'. * - * @param mixed $value The value to check. + * @param mixed $actual The value to check. + * @param mixed $message Error message to use if the assertion fails. * @return bool mixed True if the value is of integer type, false otherwise. */ - protected function assertIsInt( $value ) { - return $this->assertInternalType( 'int', $value ); + public static function assertIsInteger( $actual, $message = '' ) { + return self::assertInternalType( 'int', $actual, $message ); } } diff --git a/tests/php/includes/wc-stock-functions-tests.php b/tests/php/includes/wc-stock-functions-tests.php index 74470a86cbc..4e04dc29d29 100644 --- a/tests/php/includes/wc-stock-functions-tests.php +++ b/tests/php/includes/wc-stock-functions-tests.php @@ -200,7 +200,7 @@ class WC_Stock_Functions_Tests extends \WC_Unit_Test_Case { */ private function assertIsIntAndEquals( $expected, $actual ) { $this->assertEquals( $expected, $actual ); - $this->assertIsInt( $actual ); + self::assertIsInteger( $actual ); } /**