Skip to content
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

video-* plugins: stim parameter name change and tests #1261

Merged
merged 8 commits into from Dec 3, 2020

Conversation

becky-gilbert
Copy link
Collaborator

These are the changes from @mgorenstein's pull request (#1171), merged into this branch so that I could also update the documentation and example HTML files with the new parameter name.

I also added tests to each of the plugin test files to make sure that (1) the plugin files are calling the registerPreload function, and (2) the parameter name that is used in that function exists in the plugin (to catch the stimulus/sources name error that was preventing video preloading before).

  1. The easiest way I could think of to test whether an incorrect parameter name was used in jsPsych.pluginAPI.registerPreload was to modify jspsych.js so that it produces a console warning if the parameter in the preloads list is undefined, and then detect this console warning in the test. This test is working, i.e. it fails when jsPsych.pluginAPI.registerPreload uses a parameter name that doesn't exist in the trial object and passes when stimulus is used as the parameter name.

  2. I couldn't get the jest.spyOn() function to work for spying on jsPsych.pluginAPI.registerPreload. I thought that this would work:const preload_spy = jest.spyOn(jsPsych.pluginAPI, 'registerPreload'); //... set up and run video trial expect(preload_spy).toHaveBeenCalled(); But this always fails, saying that preload_spy was not called. I kept this in the code but commented out the expect line to make the tests pass. @jodeleeuw any ideas/suggetions?

Also @jodeleeuw I realize that this sort of testing isn't quite what you meant in this comment, but figured this was the quickest way to see if I can get the preload tests working. I can test this in a different way and move the tests somewhere else if you prefer. I think including preload tests somewhere will be useful for #1258.

@becky-gilbert becky-gilbert added this to the 6.2 milestone Dec 2, 2020
@jodeleeuw
Copy link
Member

I wonder if the reason that .spyOn isn't working is because the plugin file is loading before the spy is created. The registerPreload method is called when the plugin is loaded for the first time. I believe in most of the tests we load the plugin file before running the test. Maybe for this test we need to load jsPsych, set up the spy, and then load the plugin.

@becky-gilbert
Copy link
Collaborator Author

@jodeleeuw yep that was it - fixed the tests now. Thanks!
Is this ok to be merged?

@becky-gilbert becky-gilbert merged commit de9149c into master Dec 3, 2020
@becky-gilbert becky-gilbert mentioned this pull request Dec 3, 2020
@jodeleeuw jodeleeuw deleted the video-plugin-testing branch September 15, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants