The code hacker needs to be reset before each test. This was done via
a couple of classes implementeing BeforeTestHook, those were registered
in phpunit.xml.
The problem is that the PHPUnit version used for WooCommerce unit test
has recently been changed from 7.5 to 6.5 for compatibility with
PHP 7.0, and hook classes were introduced in PHPUnit 7. Thus no hooks
were ran, the code hacker wasn't reset, that caused some functions
to remain hacked between tests, and this made some tests to fail.
The solution is to move the code hacker reset to the setUp method
in the base unit test class.
This commit fixes some inconsistencies in the settings pages, and
makes all the existing pages extensible by adding new sections
(that was possible in some pages, but not in others). Main changes:
1. Modify the 'get_sections' method so that it invokes a new protected
'get_own_sections' method and then triggers the
'woocommerce_get_sections_' . id filter.
This way the filter is triggered only in the base class
and not in each of the derived classes too.
2. Change the get_settings() method so that it has its signature
changed to get_settings( $current_section = '' )
in the base class and in all the derived class.
Some derived classes were already using this signature, but others
(those not having multiple sections natively) weren't, making then
effectively impossible to define multiple sections for these pages
via filters.
With this change all the section pages act consistently and allow
both adding new settings to the default "General" section
and creating new sections via filters.
3. Change the implementation of 'get_settings' in the base class
so that it searches for a 'get_settings_for_{section_id}_section'
method in the class and executes it, otherwise it executes the new
protected method get_settings_for_section( $current_section ); then
it triggers the 'woocommerce_get_settings_' . id filter.
This makes it easier to separate the code that returns the list
of filters in multiple methods, one per section, instead of using
one big if-else-else... block.
So now instead of overriding get_settings($current_section='') derived
classes need to implement get_settings_for_{$current_section}_section
for each section, or override get_settings_for_section($current_section)
or both. 'get_settings_for_section' returns an empty array by default.
Also, 'woocommerce_get_settings_' . id is triggered in one single
place too.
Other improvements:
* Remove duplicated code from 'output' in 'WC_Settings_Page' children.
Some classes inherited from 'WC_Settings_Page' override the 'output'
method with custom code, which in all cases ended up repeating the code
of the original method as a fallback. These repetitions have been
replaced with 'parent::output()'.
* Fix inconsistencies for 'save' and 'output' in WC_Settings_Tax/Emails
The 'WC_Settings_Tax' and 'WC_Settings_Emails' classes had some
inconsistencies in their 'save' and 'output' methods that prevented the
proper creation new sections and the addition of new settings via the
'woocommerce_get_sections_' and 'woocommerce_get_settings_' filters.
Now they work as expected.
* Deduplicate parts of 'save' in 'WC_Settings_Page' and children.
Two methods have been added to 'WC_Settings_Page' class:
'save_settings_for_current_section' and 'do_update_options_action'.
These are intended to be invoked by derived classes in their 'save'
methods, in order to remove code repetition.
* Add some helper methods to WC_Unit_Test_Case.
Methods added:
- assertOutputsHTML
- assertEqualsHTML
- normalize_html
- capture_output_from
After the League's Container package has been reintroduced, all the
code that implements the dependency injection mechanism in woocommerce
can be brought back as well.
In PHP 8 required parameters after optional parameters in
function/method signatures trigger a deprecation notice. These type
of parameters are pointless since a value needs to always be
provided for them anyway, so they are actually de-facto required.
This commit converts all these not-so-optional parameters into
truly required parameters by removing their default values.
Since reverting the PR at this point would be a mess I've gone ahead and removed the internals of the container. We should aim to keep the class since it's now part of our public API but it won't work as expected anymore. This is fine for now since we don't actually have anything in it!
Now, if there are filters present the logic is as follows:
- For multiple filtering values of the same attribute:
the product is visible if there's at least one variation
that has one of the filtering values associated to the attribute,
or if there's at least one variation having the attribute
with a value of "Any".
- For filtering by more than one attribute:
the product is visible if there's at least one variation that
is visible for ALL the attributes according to the above rule.
Note that this is irrespective of the type of logic configured for
the filter (OR or AND).
The layered nav filtering doesn't work well with variable products
when some variations have stock and other don't. When a term is
selected in the widget, a variable product having no stock for
the variation corresponding to that term but having stock for
other variations will be displayed, but it shouldn't.
This commit fixes that by introducing two changes:
- A new override of "is_visible" for WC_Product_Variable that
looks at the supplied filters, compares them against the corresponding
available variations and calculates the visibility based on
the query type (OR or AND).
- A hook on the "found_posts" filter in WC_Query, that adjusts
the posts count based on the found products visibility
when there are filters available; this is needed to sync the
"displaying X posts" messages and the paging when variable
products are hidden due to stock status.
Additionally, the visibility calculated in "found_posts" is cached
as loop variables so that it isn't calculated again when actually
displaying the products.
- Method and class renames.
- Removed unnecessary autoloader registration.
- Add a unit test for classes with non-object type hints
in constructor arguments.
- camelCase methods changed to snake_case for consistency with WP.
- Added a check in `ExtendedContainer::get` that throws an informative
exception if a non-namespaced class name is passed.
- `container->reset_resolved()` is called during unit testing bootstrap.
- Added some utility methods in `WC_Unit_Test_Case`.
- Added a new class `ExtendedContainer` that extends League's container.
- `add` modified to reject classes not in the root Woo namespace.
- Has two new methods, `replace` and `reset_resolved`.
- It's used as the underlying container instead of League's one
in `Container`, but the new methods are not exposed.
- At unit test bootstrap time the globally registered container is
replaced with the extended one that `Container` stores
(grabbed from private property using reflection).
- A new `MockableLegacyProxy` is added. It inherits from `LegacyProxy`
and allows to mock functions, static methods and legacy classes.
- The registeed `LegacyProxy` is replaced with the mockable version
during unit test bootstrap.
- A PHPUnit hook is added to reset the mockable proxy to its initial
state (so that nothing is mocked) before each test.
- `WC_Unit_Test_Case` gets helper methods to mock functions, static
methods and classes without having to retrieve the proxy class.
Those methods are a convenient replacement for
"this->factory->user->create". Tests that were using that to
simulate user login have been modified to use the new methods.
The testing tools (only the code hacker at this time) have been moved
from 'src' to 'tests/Tools', since many opcode cache plugins
load the whole src folder in production.
Also, an extra autoloader is set in the tests bootstrap so that
the 'tests/Tools' directory corresponds, using PSR4, to the
'Automattic\WooCommerce\Testing\Tools' namespace.
- Add methods to temporarily disable and reenable the code hacker.
The code hacker is causing issues in some tests that perform
write operations to the local filesystem. Since this happens only
in a few cases, the easiest fix is to temporarily disable the
code hacker when that happens. This commit adds two new methods
for that in `WC_Unit_Test_Case`: `disable_code_hacker` and
`reenable_code_hacker`.
These methods use a disabling requests count so that the hacker
isn't enabled before it should. E.g. you call `disable`, then
a helper method that does `disable` and `enable`, then `enable` -
then only the last `enable` will have effect.
- `CodeHacker::add_hack` has now a boolean `persistent` parameter.
Persistent hacks won't be cleared by `clear_hacks`.
- `CodeHackerTestHook::executeAfterTest` will now disable the hacker
only if no persistent hacks are registered.
- The existing `file_copy` method is made static for consistency.
- `CodeHacker::restore` method renamed to `disable` for clarity.
- Fix how CodeHackerTestHook::executeBeforeTest parses the test name,
to account for warnings and tests with data sets.
- CodeHackerTestHook now includes a executeAfterTest hook that
disables the code hacker (needed to prevent it from inadvertently
altering further tests). Also, clear_hacks is executed in
executeBeforeTest for the same reason.
- CodeHacker gets restore, clear_hacks and is_enabled methods
to support the changes in CodeHackerTestHook.
- FunctionsMockerHack fixed so that it doesn't modify strings
that are class method definitions.
- Added the WC_Unit_Test_Case::file_copy method, it must be used
instead of the PHP built-in "copy" in tests, otherwise tests
that run with the code hacker active will fail.
This is something to investigate.
Nestor Soriano
Nestor Soriano
vedanshujain
Christopher Allford
Claudio Sanches