Make WordPress Core

Opened 4 years ago

Last modified 4 years ago

#51446 new defect (bug)

CORS issues with QUnit while running tests

Reported by: garrett-eclipse's profile garrett-eclipse Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.5
Component: Build/Test Tools Keywords: has-patch needs-testing dev-feedback has-unit-tests needs-refresh
Focuses: javascript Cc:

Description

Hello,

For a while I've been having an error when running grunt test with the qunit tests;

Running "qunit:files" (qunit) task
Testing tests/qunit/compiled.html ......................................................................................................................................................................................................................................................................................................OK
Testing tests/qunit/index.html Access to XMLHttpRequest at 'file:///wp-admin/admin-ajax.php' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, https.
Failed to load resource: net::ERR_FAILED
......................................................................................................................................................................................................................................................................................................OK
>> 588 tests completed with 0 failed, 0 skipped, and 0 todo.
>> 1738 assertions (in 8024ms), passed: 1738, failed: 0

Looking into this a little I was able to resolve these by updating the tests/qunit/index.html to use localhost for the ajax urls.

Uploading patch but am unsure if it's the correct action, seems to work nicely for me at least and suppresses the error.

Attachments (2)

51446.diff (872 bytes) - added by garrett-eclipse 4 years ago.
Specifically use localhost in the qunit tests to avoid CORS issues from localost to file:///
Screen Shot 2020-10-07 at 4.53.42 PM.png (252.7 KB) - added by garrett-eclipse 4 years ago.
New JQMIGRATE issues in qunit tests.

Download all attachments as: .zip

Change History (19)

@garrett-eclipse
4 years ago

Specifically use localhost in the qunit tests to avoid CORS issues from localost to file:///

#1 @garrett-eclipse
4 years ago

One other note, going directly to the qunit index.html file in the browser had all tests passing properly without CORS issue. It was only via command line that it threw the errors.

This ticket was mentioned in Slack in #core by garrett-eclipse. View the logs.


4 years ago

#3 @desrosj
4 years ago

  • Component changed from General to Build/Test Tools

Thanks @garrett-eclipse! Can you give a bit of information around your set up? I'm curious why this has not been encountered before.

#4 @garrett-eclipse
4 years ago

Hey @desrosj I'm on macOS Catalina (10.15.6), using MAMP 5.7 and SVN 1.10.4.
NPM v6.14.8
GRUNT-cli v1.3.2
GRUNT v1.1.0
QUnit v2.9.3

Steps to run tests (skipping other unrelated steps);

  1. Create folder.
  2. Install via SVN.
  3. Run npm install
  4. Rename wp-tests-config-sample.php to wp-tests-config.php
  5. Create DB, update wp-tests-config.php
  6. Run grunt test

*This is just going straight to testing, I often patch or build but always same CORS issue when running the tests.

Note: I mentioned on Slack a couple times the last couple months so feels like it started since 5.5.

#5 @iandunn
4 years ago

Those lines where introduced in r37714 and r40640. I'm guessing they intentionally used relative URLs, so that people could run it from any host. It'd be great to keep that flexibility if we can.

rm -f compiled.html fixed a similar problem for me. At a quick glance, grunt should do that, but there may be some edge cases where it gets stuck and needs to be done manually. If your copy is outdated, that could be the cause.

  • What URL do run Core from normally?
  • Can you share your Apache vhost config for the site?

#6 @iandunn
4 years ago

Hmm, I just saw this error in Travis, even though I'm not getting it locally.

#7 @garrett-eclipse
4 years ago

Thanks @iandunn, I'm not alone ;)

That being said, while coming back to look at this I ran grunt test before I realized MAMP was enabled... and it ran... reminding me I have Laravel Valet running on localhost itself and MAMP is localhost:8888.
That being said opening the qunit file directly using either localhost or localhost:8888 to be served by either MAMP or Valet the tests work fine. It's only via command line this seems to be an issue.
*Mainly mentioning to remove direct relation to MAMP there.

From searches I wonder if we enable --disable-web-security similar to seen here for the qunit setup in Grunt.js, example;
https://github.com/gruntjs/grunt-contrib-qunit/issues/158#issuecomment-511819334

Is there a need for CORS checks during qunit testing?

This ticket was mentioned in PR #574 on WordPress/wordpress-develop by iandunn.


4 years ago
#8

  • Keywords has-unit-tests added

Add QUnit options to disable CORS.

Trac ticket: https://core.trac.wordpress.org/ticket/51446

iandunn commented on PR #574:


4 years ago
#9

  • [ ] The CORS error is fixed, but now there's a new one:

{{{sh
Running "qunit:files" (qunit) task
Testing tests/qunit/compiled.html .................................................................................................................................................................................................................................................................................................................OK
Testing tests/qunit/index.html Failed to load resource: net::ERR_FILE_NOT_FOUND
.................................................................................................................................................................................................................................................................................................................OK
}}}

  • [ ] I'm not sure if all 3 options are needed, or if just --disable-web-security would be enough

#10 @iandunn
4 years ago

Ah, yeah we updated to grunt-contrib-qunit 3.1.0 in #49768.

I am able to reproduce this locally now, even on master. I just have to run grunt qunit:compiled instead of grunt qunit.

Your suggestion worked, but I'm now getting the new error above.

#11 @garrett-eclipse
4 years ago

  • Keywords needs-refresh added

Nice thanks for the PR @iandunn that fixed the CORS for me, I tried dropping the extra params so it was just --disable-web-security and that was all that the CORS issue needed like;

qunit: {
	options: {
		puppeteer: {
			args: [ "--disable-web-security" ]
		}
	},

But I am also getting the error for Failed to load resource: net::ERR_FILE_NOT_FOUND. Looking into that, seems it's a missing file or incorrect reference.

#12 @garrett-eclipse
4 years ago

I've looked at all script references without luck they all seem correct and exist. It's unfortunate we can't see which file isn't found. Is there a way to make the error more verbose and give us the path of the resource?

@garrett-eclipse
4 years ago

New JQMIGRATE issues in qunit tests.

#13 follow-up: @garrett-eclipse
4 years ago

  • Version set to trunk

Hmm, this may be due to [49101]/#50564 recently committed by @azaozz but now on a clean trunk running the qunit tests my CORS issue and that Failed to load resource issue are gone and in their place several jQuery and JQMIGRATE messages;

Running "qunit:files" (qunit) task
Testing tests/qunit/compiled.html JQMIGRATE: Migrate is installed, version 3.3.1
.........................................................................................................................................................................................................OK
Testing tests/qunit/index.html JQMIGRATE: Migrate is installed with logging active, version 3.3.1
JQMIGRATE: jQuery.expr[':'] is deprecated; use jQuery.expr.pseudos
console.trace
JQMIGRATE: jQuery.isArray is deprecated; use Array.isArray
console.trace
JQMIGRATE: jQuery.isFunction() is deprecated
console.trace
JQMIGRATE: jQuery.trim is deprecated; use String.prototype.trim
console.trace
JQMIGRATE: jQuery.fn.click() event shorthand is deprecated
console.trace
JQMIGRATE: jQuery.fn.bind() is deprecated
console.trace
JQMIGRATE: jQuery.unique is deprecated; use jQuery.uniqueSort
console.trace
JQMIGRATE: jQuery.fn.keydown() event shorthand is deprecated
console.trace
JQMIGRATE: jQuery.fn.scroll() event shorthand is deprecated
console.trace
JQMIGRATE: jQuery.fn.focus() event shorthand is deprecated
console.trace
.........................................................................................................................................................................................................OK
>> 402 tests completed with 0 failed, 0 skipped, and 0 todo.
>> 1002 assertions (in 7800ms), passed: 1002, failed: 0

Note these are also present in the inspect console if you open the qunit files directly;
https://core.trac.wordpress.org/raw-attachment/ticket/51446/Screen%20Shot%202020-10-07%20at%204.53.42%20PM.png

#14 @iandunn
4 years ago

Is there a way to make the error more verbose and give us the path of the resource?

grunt qunit --verbose doesn't help :(


now on a clean trunk running the qunit tests my CORS issue and that Failed to load resource issue are gone and in their place several jQuery and JQMIGRATE messages;

Huh. I'm also getting other errors, like these:

global failure...ERROR
>> Message: Uncaught TypeError: o.widget is not a function
>> Actual: null
>> Expected: undefined
>> TypeError: o.widget is not a function
>>   at file:///Users/iandunn/vhosts/localhost/wp-develop.test/public_html/build/wp-includes/js/jquery/ui/mouse.min.js:11:163
>>   at file:///Users/iandunn/vhosts/localhost/wp-develop.test/public_html/build/wp-includes/js/jquery/ui/mouse.min.js:11:84
>>   at file:///Users/iandunn/vhosts/localhost/wp-develop.test/public_html/build/wp-includes/js/jquery/ui/mouse.min.js:11:94

tinymce.obsolete - global failure...ERROR
>> Message: Uncaught TypeError: this.$sectionContent.sortable is not a function
>> Actual: null
>> Expected: undefined
>> TypeError: this.$sectionContent.sortable is not a function
>>   at child._setupSortable (file:///Users/iandunn/vhosts/localhost/wp-develop.test/public_html/build/wp-admin/js/customize-widgets.js:1897:25)



I'm seeing similar problems with job 396605514 (for #github539.

But, I'm not seeing any for job 396636757 for #github576 (which didn't change any qunit files).


I tried updating qunit, etc in #github574 (related #50769), but that didn't help.

cc @whyisjake, @SergeyBiryukov, in case you have any ideas.

#15 in reply to: ↑ 13 @azaozz
4 years ago

Replying to garrett-eclipse:

Hmm, this may be due to [49101]/#50564...

Right. All the JQMIGRATE: warnings are things that need fixing. They show up because jQuery Migrate 3.3.1 is loaded in debug more (same as with SCRIPT_DEBUG). Most are pretty trivial, replace jQuery( ... ).click() with jQuery( ... ).on( 'click', etc. They are safe to ignore for now, but ideally should be fixed :)

Huh. I'm also getting other errors, like these...

Most likely caused by caches. ui/widget.js is now part of ui/core.js. There were few timing errors in some tests, fixed here: https://core.trac.wordpress.org/changeset/49101#file49.

Last edited 4 years ago by azaozz (previous) (diff)

iandunn commented on PR #574:


4 years ago
#16

I removed allow-file-access-from-files per https://core.trac.wordpress.org/ticket/51446#comment:11. I kept --headless, though, because I got a Chromium popup after removing it.

#17 @helen
4 years ago

  • Version changed from trunk to 5.5
Note: See TracTickets for help on using tickets.