forked from gocodebox/lifterlms
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add module loader initial tests #2
Open
eri-trabiccolo
wants to merge
2
commits into
actual-saurabh:feature/cert-builder
Choose a base branch
from
eri-trabiccolo:feature/cert-builder
base: feature/cert-builder
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
tests/unit-tests/modules/class-llms-test-module-loader.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
<?php | ||
/** | ||
* Tests for LifterLMS Module loader | ||
* | ||
* @group modules | ||
* | ||
* @since [version] | ||
* @version [version] | ||
*/ | ||
class LLMS_Test_Module_Loader extends LLMS_UnitTestCase { | ||
|
||
/** | ||
* Test the modules loading. | ||
* | ||
* @since [version] | ||
* | ||
* @return void | ||
*/ | ||
public function test_load() { | ||
|
||
// get modules to be loades. | ||
$modules = $this->_get_load_info(); | ||
|
||
// check they've been loaded. | ||
foreach ( $modules as &$module ) { | ||
$module['actions'] = did_action( "lifterlms_module_{$module['name']}_loaded" ); | ||
$this->assertEquals( 1, $module['actions'] ); | ||
LLMS_Module_Loader::instance()->is_module_loaded( $module['name'] ); | ||
} | ||
|
||
// define module constants to false so to skip the loading. | ||
foreach ( $modules as $module ) { | ||
! defined( $module['constant_name'] ) && define( $module['constant_name'], FALSE ); | ||
} | ||
|
||
// Fire the loading once again. | ||
$loaded = LLMS_Unit_Test_Util::call_method( LLMS_Module_Loader::instance(), 'load' ); | ||
|
||
// check they've not been loaded this time. | ||
$this->assertEquals( array(), $loaded ); | ||
|
||
foreach ( $modules as &$module ) { | ||
$actions_counter = $module['actions']; | ||
$module['actions'] = did_action( "lifterlms_module_{$module['name']}_loaded" ); | ||
$this->assertEquals( $actions_counter, $module['actions'] ); | ||
} | ||
|
||
} | ||
|
||
/** | ||
* Get the array of modules to load. | ||
* Of the type: | ||
* $module = array( | ||
* 'name' => 'module-name', | ||
* 'file_path' => 'lifterlms/includes/modules/module-name/class-llms-module-name.php', | ||
* 'constant_name' => 'LLMS_MODULE_NAME', | ||
* ); | ||
* | ||
* @since [version] | ||
* | ||
* @return array | ||
*/ | ||
private function _get_load_info() { | ||
return LLMS_Unit_Test_Util::call_method( LLMS_Module_Loader::instance(), 'load_info' ); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I'm having difficulties in understanding. Is there a way I can write separate tests,
test_load()
andtest_do_not_load()
? Because there are two possible scenarios in the real use:Both these scenarios need to work and therefore, need to be tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a dummy module within the test suite and filter https://github.com/actual-saurabh/lifterlms/blob/feature/cert-builder/includes/modules/class-llms-module-loader.php#L111 to remove every other module and supply this dummy's information instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, probably my comments in the code are not clear. Let me explain:
The code above checks both that the modules are loaded if no constant is defined (which is the default) and that the modules are NOT LOADED when defining the respective constants to false (see the piece of code that starts from
// define module constants to false so to skip the loading.
).I see understanding what that test does and why it does it that way can be tricky, but let me try to explain:
The unit tests bootstrapping right now is done in a way that the actual unit test classes are loaded AFTER WordPress is loaded, so when the method above
test_load()
is executed, the plugin'sLLMS_Module_Loader
class has been already instantiated.So
LLMS_Module_Loader::$instance->load()
has already been executed once. So what I can do is just checking the modules I expected to be loaded have been actually loaded, and I check that with the "tools" theLLMS_Module_Loader
gives me: it fires specific actions when modules are loaded, and I check thatdid_action('action')
is equal to the number of times that action has been triggered, in our case, we expect that equals to 1.Then I want to test that setting a specific constant to false will prevent a module from being loaded. How can I?
(Remember that, as of now, for what I said above about the unit tests bootstrapping etc., I have no to prevent the first
load
.)I define the modules constants as false and emulate a new load by manually calling the
LLMS_Module_Loader::$instance->load()
method, and then check that:LLMS_Module_Loader::$instance->load()
is an empty array.did_action('action')
for each module is the same as before => meaning that the action has not been triggered this time.Still because of the bootstrapping thing, to perform the tests based on the constants, it's mandatory that we don't define those constants the first time we trigger the
LLMS_Module_Loader::$instance->load()
method. I cannot undefine a constant, and I cannot perform any before theLLMS_Module_Loader::$instance->load()
method is fired the first time. The tricky part of the unit tests is that we have to write code that can be tested. :Dp.s.
to do something before wp and lifterlms plugins are loaded we have to act on the bootstrapping file here (the
load
method):https://github.com/gocodebox/lifterlms/blob/3d088e89b41e59641dab1e86ca948b6bfd0087fe/tests/bootstrap.php
But you can see that 'place' is pretty generic and defining specific constants for a specific test there is not ideal to me.
Yes we can, sure, we can add them here:
https://github.com/gocodebox/lifterlms/tree/master/tests/assets
Actually I think we can
touch
and thenremove
files with php api. I'm going to look into this and add a new commit to this PR.Hope I've been clear. Let me know if otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added dummy modules tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will it make things better if I remove these lines from the module loader: https://github.com/actual-saurabh/lifterlms/blob/feature/cert-builder/includes/modules/class-llms-module-loader.php#L118-L126 and shift the responsibility of setting up and checking the constant into the module itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the problem would still be the same.
Once a constant is defined we cannot "unset" it. And since a first load happen before our tests start the constant gets defined... and we're back to square one.