Add oEmbed provider unit tests

[johnbillion]

A unit test should be added which tests each of our supported oEmbed providers to make sure that each provider returns the expected response.

Ideally the tests would be in their own group which is excluded from a default test run, in the same way AJAX tests work. The tests should test the HTTP response code and the validity of the JSON and XML endpoints where appropriate.

[voldemortensen]

I'll start working on this tomorrow.

[johnbillion]

Thanks voldemortensen. If you need media URLs for any of the providers, #28507 should have one for each.

[johnbillion]

32360.patch​ introduces tests for the responses returned by each of our oEmbed providers. The tests which actually request the oEmbed for each of the test URLs is in the external-oembed group which doesn't run by default (these can be run with the --group=oembed or --group=external-oembed CLI option).

Currently missing from the tests:

• Test URL(s) for poll.fm.
• Test URL(s) for video214.com.
• Test URL(s) for gi*.photobucket.com (*cough* #28379 *cough*).

Currently failing:

• scribd.com is missing the width attribute.
• polldaddy.com is missing the width attribute.
• meetup.com returns an incorrect content-type header.
• animoto.com returns an incorrect content-type header.
• tumblr.com doesn't respect the maxwidth parameter for its width attribute.
• cloudup.com doesn't respect the maxwidth parameter for its thumbnail_width attribute.
• reverbnation.com doesn't adhere to the oEmbed specification (see #33207).

[johnbillion]

Other things I'd like to test:

• Check the correct URL scheme is used in the response when https URLs are requested.
• Check invalid maxwidth and maxheight parameters are handled correctly (background: 29126#comment:9).
• Check HTTPS support? (See #28507)

[johnbillion]

Latest patch, 32360.2.patch​, tests everything I want to cover. Twelve failures. Bleugh.

[netweb]

Related: #34651 Instagram URLs with www will not embed

With that, the tests here for Instagram should be updated to include the w's per r35634

[wonderboymusic]

In 38454:

OEmbed: add unit tests. @group external-oembed is not run by default.

Props johnbillion, wonderboymusic.
See #32360.

[jorbin]

@wonderboymusic @johnbillion Did you intend for the grunt task to be commented out? If so, why add it?

[johnbillion]

The testOembedTestsCoverAllProviders() test has been altered from my patch and now doesn't work as intended. It should fail when a new provider is added to WP_oEmbed->providers, which it doesn't.

[johnbillion]

In 38512:

Embeds: Update the oEmbed provider test suite.

• Remove the manual flag for HTTPS support and replace it with a simple check on the URL format.
• Ensure testOembedTestsCoverAllProviders() actually fails when a new provider is added without a corresponding test.

See #32360

[johnbillion]

So the reason these tests hadn't gone into core up until this point is because there are so many failures with the external providers.

The tests are quite volatile too (I can see at least two tested URLs which are 404ing that previously weren't), for example Kickstarter is sending me 429 Too Many Requests responses, and Cloudup is sending me 502 Bad Gateway intermittently.

I guess it makes sense for the tests to remain in a group which never gets tested by default (ie. without passing the --group external-oembed flag), but then I'm not sure if that's super valuable.

In addition, the testOembedProviderReturnsExpectedResponse() test really needs to be split into several smaller tests so that every assertion can be performed (as opposed to the test failing at the first assertion failure). This means moving the actual HTTP requests into a data provider.

[johnbillion]

In 38514:

Embeds: Clarify some assertion failure messages and correct a test URL for Twitter timelines.

See #32360

[boonebgorges]

I guess it makes sense for the tests to remain in a group which never gets tested by default (ie. without passing the --group external-oembed flag), but then I'm not sure if that's super valuable.

I just ran into this, because the <exclude><group>external-oembed</group></exclude> added to phpunit.xml.dist wasn't automatically picked up by my custom phpunit.xml.

I definitely think it's wise to have tests for this stuff, and I think it's smart for them not to run by default. But if they're failing on a regular and predictable basis, then they shouldn't be in core (or perhaps they should be commented out in the dataProvider while the details are ironed out).

[ocean90]

In 38692:

Embeds: Update the oEmbed provider test suite.

• Dailymotion: Use an URL for an existing video.
• Facebook: Remove URLs which don't exist and update provider map for [38691].

See #32360.

[johnbillion]

In 38840:

oEmbed: Remove the oEmbed provider unit tests.

This reverts [38454] along with its follow-up commits, [38512], [38514], and [38692]. These tests are currently not pass
ing, and maybe they never will. The tests are in a group which does not run by default without a flag, making them quest
ionably useful.

We can re-visit this at a later date.

See #32360