Skip to content

Commit f13b86f

Browse files
sol-loupfacebook-github-bot
authored andcommitted
fix skipped tests2 (#3058)
Summary: ## Description This PR addresses noisy error logs originating from `WC_Facebookcommerce_Utils::fblog` during PHPUnit tests in `WCFacebookCommerceIntegrationTest`. These errors occurred because the `wc_facebook_external_merchant_settings_id` option (External Merchant Settings ID) was not set during test execution, leading to false positive error messages like "external merchant setting is null". The fix involves initializing this option with a dummy value (`'dummy_ems_id'`) within the `setUp()` method of the `WCFacebookCommerceIntegrationTest` class. This ensures the setting is present before relevant tests run, preventing the erroneous `fblog` calls. Additionally, this PR includes the following test cleanup: - Deleted the test `test_allow_full_batch_api_sync_uses_allow_full_batch_api_sync_filter` as it was testing a deprecated option (`allow_full_batch_api_sync`) and was already marked as skipped. - Removed `markTestSkipped` annotations from `test_get_js_sdk_version_returns_id_from_options_using_no_filter` and `test_get_js_sdk_version_returns_id_with_filter` as they now pass successfully. ### Type of change - Bug fix (non-breaking change which fixes an issue) - Syntax change (non-breaking change which fixes code modularity, linting or phpcs issues) ## Checklist - [] I have commented my code, particularly in hard-to-understand areas. - [] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [] I followed general Pull Request best practices. Meta employees to follow this [wiki](https://fburl.com/wiki/2cgfduwc). - [] I have added tests (if necessary) and all the new and existing unit tests pass locally with my changes. - [] I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality. - [] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki](https://fburl.com/wiki/nhx73tgs). ## Changelog entry Fix - Prevent spurious error logs during unit tests by initializing external merchant settings ID. Fix - Remove obsolete/skipped unit tests and enable previously skipped tests that now pass. Pull Request resolved: #3058 Test Plan: Imported from GitHub, without a `Test Plan:` line. **!---- (auto-generated) DO NOT EDIT OR PUT ANYTHING AFTER THIS LINE ----!** MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: www / Diff Version V1 https://internalfb.com/intern/testinfra/testrun/6192449709976434 MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: bloks / Diff Version V1 https://internalfb.com/intern/testinfra/testrun/14073748913725649 Reviewed By: carterbuce Differential Revision: D72987618 Pulled By: sol-loup fbshipit-source-id: 8e0762d8462ddbe95a31555c061d53e3fdd931f5
1 parent bfaaa60 commit f13b86f

File tree

1 file changed

+2
-27
lines changed

1 file changed

+2
-27
lines changed

tests/Unit/WCFacebookCommerceIntegrationTest.php

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ public function setUp(): void {
7070
/* Making sure no options are set before the test. */
7171
delete_option( WC_Facebookcommerce_Pixel::SETTINGS_KEY );
7272
delete_option( WC_Facebookcommerce_Integration::SETTING_FACEBOOK_PIXEL_ID );
73+
// Needed to prevent error logs in tests.
74+
WC_Facebookcommerce_Utils::$ems = 'dummy_ems_id';
7375
}
7476

7577
/**
@@ -417,29 +419,6 @@ function ( bool $status ) {
417419
$this->assertFalse( has_filter( 'facebook_for_woocommerce_allow_full_batch_api_sync' ) );
418420
}
419421

420-
/**
421-
* Tests default allow full batch api sync uses facebook_for_woocommerce_allow_full_batch_api_sync filter
422-
* to overwrite allowance status.
423-
*
424-
* @return void
425-
*/
426-
public function test_allow_full_batch_api_sync_uses_allow_full_batch_api_sync_filter() {
427-
$this->markTestSkipped( 'Some problems with phpunit polyfills notices handling.' );
428-
429-
$this->add_filter_with_safe_teardown(
430-
'facebook_for_woocommerce_allow_full_batch_api_sync',
431-
function ( bool $status ) {
432-
return false;
433-
}
434-
);
435-
436-
$status = $this->integration->allow_full_batch_api_sync();
437-
438-
$this->assertFalse( $status );
439-
$this->assertFalse( has_filter( 'facebook_for_woocommerce_block_full_batch_api_sync' ) );
440-
$this->assertTrue( has_filter( 'facebook_for_woocommerce_allow_full_batch_api_sync' ) );
441-
}
442-
443422
/**
444423
* Tests plugin enqueues scripts and styles for non admin user for non plugin settings screens.
445424
*
@@ -2177,8 +2156,6 @@ function ( $pixel_install_time ) {
21772156
* @return void
21782157
*/
21792158
public function test_get_js_sdk_version_returns_id_from_options_using_no_filter() {
2180-
$this->markTestSkipped( 'get_js_sdk_version method is called in constructor which makes it impossible to test it in isolation w/o refactoring the constructor.' );
2181-
21822159
add_option( WC_Facebookcommerce_Integration::OPTION_JS_SDK_VERSION, 'v1.0.0' );
21832160
$this->teardown_callback_category_safely( 'wc_facebook_js_sdk_version' );
21842161

@@ -2193,8 +2170,6 @@ public function test_get_js_sdk_version_returns_id_from_options_using_no_filter(
21932170
* @return void
21942171
*/
21952172
public function test_get_js_sdk_version_returns_id_with_filter() {
2196-
$this->markTestSkipped( 'get_js_sdk_version method is called in constructor which makes it impossible to test it in isolation w/o refactoring the constructor.' );
2197-
21982173
$this->add_filter_with_safe_teardown(
21992174
'wc_facebook_js_sdk_version',
22002175
function ( $js_sdk_version ) {

0 commit comments

Comments
 (0)