Dont bundle wp scripts

[mattheu]

The JS tests rely on some core WP JS. The solution so far was to bundle a copy of the scripts.

This is obviously a pain.
They get out of date easily.
We depend on quite a few, and may need more in future.

I'd much rather just reference the core files so we can run the tests against whichever version of WP you have.

PROBLEM - how to determine the relative path to these files? I've used an abspath variable in the Gruntfile, which you can modify as needed. EG if Shortcake is bundled in a theme on WP VIP.

Is there a better solution to this problem? Should I just keep bundling the files and keep them all updated?

Would love some thoughts on this.

[mattheu]

One idea - we can use the test WP install used by PHP unit.

This does add a barrier to running the tests though, so I've removed them from the standard 'scripts' task. We will have it running on Travis so I think this is OK.

[mattheu]

OK I think I've got a solution. Use grunt option to pass the custom path.

eg
grunt jasmine --abspath="/srv/www/wp-test" to run the tests using a test install.
grunt jasmine --abspath="../../../" to run the tests using the current install, if shortcake is in the plugins directory.

It will default to /tmp/wordpress

[mattheu]

This is updated and will read the location from the environment variable if not explicitly passed.

Also note that the test for dashes in an attribute was previously unsupported by WordPress, but using the scripts from core means we've got this fix.

Anything further need doing here?

[danielbachhuber]

I think this needs to be process.env.WP_DEVELOP_DIR + '/src/'

$ echo $WP_DEVELOP_DIR
/srv/www/wordpress-develop.dev/

[danielbachhuber]

I get this error when I try to run the tests locally:

local ➜  shortcake git:(dont-bundle-wp-scripts) ✗ grunt jasmine
/srv/www/wordpress-develop.dev/src
Running "jasmine:shortcodeUI" (jasmine) task
Testing jasmine specs via PhantomJS

>> TypeError: 'undefined' is not an object (evaluating 'Backbone.Model') at
>> js-tests/build/specs.js:557
>> js-tests/build/specs.js:568
>> js-tests/build/specs.js:1 s
>> js-tests/build/specs.js:1
>> js-tests/build/specs.js:2
>> js-tests/build/specs.js:1 s
>> js-tests/build/specs.js:1 e
>> js-tests/build/specs.js:1210
Warning: No specs executed, is there a configuration error? Use --force to continue.

What am I doing wrong?

[mattheu]

I've updated this to include a task that will perform the check, and throw a fatal if the path isn't found.

I've also added a grunt tests command that will run this check first.

Finally I've updated the docs to indicate how to run these tests.

[mattheu]

@danielbachhuber I'd like to get this one live - so am interested in checking whether you are still having problems.

Regarding your error, it looks like WP core files are not found. If you run grunt test instead of grunt jasmine there is an additional check to ensure the location is correct. I'm don't think its possible to hook this in on the standard grunt jasmine command. We will document this.

The current logic is as follows

• pass path like so: grunt test --abspath="/path/to/wp/install"`
• Reads environment variable WP_CORE_DIR
• Reads environment variable WP_DEVELOP_DIR and looks in WP_DEVELOP_DIR + '/src'
• Defaults to /tmp/wordpress

Let me know if you think this needs to be improved.

[danielbachhuber]

Let me know if you think this needs to be improved.

Given the additional complexity involved in getting the test suite to run, I still think it's better to just copy the core files into the test suite.

[mattheu]

OK. The big problem with this is keeping them up to date with the latest code, as there's quite a few and we may need even more in future. How about a grunt task to automate updating them instead? We can just commit these changes.

[danielbachhuber]

How about a grunt task to automate updating them instead? We can just commit these changes.

💯

I think this is a much better solution to the general problem. This way, it's just us that have to deal with the burden of the dependency, not everyone that wants to contribute / write tests.

[mattheu]

Updated the PR to add this task and include all the necessary scripts.

[danielbachhuber]

Does this still need to run for js-tests?

[danielbachhuber]

@mattheu Noted a few things

[mattheu]

Updated to fix issues - I knew I should have just closed this and opened a new PR ;)

[danielbachhuber]

Updated to fix issues - I knew I should have just closed this and opened a new PR ;)

:)