Skip to content

Sync some unit tests up with WPCOM versions.#6927

Merged
georgestephanis merged 18 commits intomasterfrom
wpcom/sync-unit-tests
Apr 21, 2017
Merged

Sync some unit tests up with WPCOM versions.#6927
georgestephanis merged 18 commits intomasterfrom
wpcom/sync-unit-tests

Conversation

@georgestephanis
Copy link
Copy Markdown
Contributor

This brings some of our unit tests back into sync with the copies that had been synced to wpcom by @enejb some time ago, in r106815-wpcom

Some will need to be tidied or changed before merging back in to Jetpack (some of the wpcom change to blog stuff, for example)

lancewillett and others added 11 commits April 5, 2017 16:32
…r escaping.

Also refresh unit test and add more cases to it.

See #7620.

Merges r129112-wpcom.
Conflicts:
	tests/php/modules/shortcodes/test_class.slideshow.php
Conflicts:
	tests/php/modules/shortcodes/test_class.twitter-timeline.php
…e `extract` usage.

Also adds more unit tests.

See #7620.

Merges r129257-wpcom.
Conflicts:
	tests/php/modules/shortcodes/test_class.vimeo.php
@georgestephanis
Copy link
Copy Markdown
Contributor Author

I'd also like to add back in the props for @scotchfield -- they were removed wpcom side in some tidying, but no reason to not keep them in place.

Copy link
Copy Markdown
Contributor

@lancewillett lancewillett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@scotchfield
Copy link
Copy Markdown
Contributor

That was a nice notification to receive, @georgestephanis. Thanks! ❤️

}

public function test_shortcodes_slideshow_no_js() {
switch_to_blog( 104104364 ); // test.wordpress.com
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be abstracted into a protected function setUp() or setUpBeforeClass() and run only on wpcom environments. In Jetpack, we should probably do something like manually creating db media item entries to use, instead of relying on an existing blog id.

https://phpunit.de/manual/current/en/fixtures.html


$shortcode_content = do_shortcode( $content );

$this->assertEquals( 0, strpos( $shortcode_content, '<p class="jetpack-slideshow-noscript robots-nocontent">This slideshow requires JavaScript.</p>' ) );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


$shortcode_content = do_shortcode( $content );

$this->assertEquals( ! false, strpos( $shortcode_content, 'class="slideshow-window jetpack-slideshow' ) );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


$shortcode_content = do_shortcode( $content );

$this->assertEquals( ! false, strpos( $shortcode_content, 'data-autostart="false"' ) );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same assertion concern as above.


$shortcode_content = do_shortcode( $content );

$this->assertEquals( ! false, strpos( $shortcode_content, 'data-autostart="true"' ) );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same assertion concern as above.


$shortcode_content = do_shortcode( $content );

$this->assertContains( '<iframe width="100%" height="450"', $shortcode_content );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be assertStringStartsWith?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is ok. I tested it with assertStringStartsWith and it works in the same way.

<embed allowscriptaccess="always" height="81" src="https://player.soundcloud.com/player.swf?url=http://api.soundcloud.com/tracks/70198773" type="application/x-shockwave-flash" width="100%"></embed>
</object>';

$shortcode_content = wp_kses_post( $content );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to set something before/after the test runs to make sure unfiltered_html is disabled?

*
* @param
*
* @return
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This param and return shouldn't be there.

Copy link
Copy Markdown
Contributor

@eliorivero eliorivero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left two minor comments but it looks good and works fine. The test intentionally skipped is correctly skipped and reported.

@georgestephanis
Copy link
Copy Markdown
Contributor Author

Merging myself as it had a review and was already tried and tested in wpcom.

@jeherve jeherve deleted the wpcom/sync-unit-tests branch April 23, 2017 19:15
jeherve added a commit that referenced this pull request Apr 24, 2017
eliorivero pushed a commit that referenced this pull request Apr 25, 2017
* Changelog: initial commit for 4.9 release.

* Changelog: add #6929

* Changelog: move old changelogs to changelog.txt

* Readme: restore deleted release post link.

The post is now live.

* Changelog: add #6853

* Changelog: add #6856

* Changelog: add #6857

* Changelog: add #6884

* Changelog: add #6885

* Changelog: add #6892

* Changelog: add #6894

* Changelog: add #6898

* Changelog: add #6899

* Changelog: add #6900

* Changelog: add #6909

* Changelog: add #6927

* Changelog: add #6947

* Chagelog: add #6958

* Changelog: add #6961

* Changelog: add #6963

* Changelog: add #6965

* Changelog: add #6986

* Changelog: add #7000

* Changelog: add #7013

* Changelog: add #7015

* Changelog: add #7019

* Changelog: add #7028

* Changelog: add #6998

* Changelog: add #6999

* Changelog: add #7044

* Changelog: add #6881

* Changelog: add #6922

* Changelog: add #6940

* Changelog: add #6962

* Changelog: add #6942

* Changelog: add #6959

* Changelog: add #7018

* Changelog: add #6948

* Changelog: add #6657

* Changelog: add #7030

* Changelog: add #7048

* Changelog: add #7031

* Changelog: add #6990

* Changelog: add #6957

* Changelog: add #7027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants