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

Sync some unit tests up with WPCOM versions. #6927

Merged
merged 18 commits into from Apr 21, 2017

Conversation

georgestephanis
Copy link
Member

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
Member 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
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
Contributor

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

@@ -24,4 +24,43 @@ public function test_shortcodes_slideshow() {
$this->assertNotEquals( $content, $shortcode_content );
}

public function test_shortcodes_slideshow_no_js() {
switch_to_blog( 104104364 ); // test.wordpress.com
Copy link
Member 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
Member 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
Member 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
Member 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
Member 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
Member 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
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
Member 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
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
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
Member 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.

None yet

6 participants