WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 5 weeks ago

Last modified 5 weeks ago

#41871 closed defect (bug) (fixed)

Code Editor: Add unit tests

Reported by: westonruter Owned by: ryotsun
Milestone: 4.9 Priority: high
Severity: normal Version:
Component: General Keywords: needs-patch
Focuses: Cc:

Description

PHPUnit tests should be added for wp_enqueue_code_editor() and WP_Widget_Custom_HTML.

See [41376] for #12423.

Attachments (5)

41871_1.diff (7.8 KB) - added by ryotsun 6 weeks ago.
41871_2.diff (8.0 KB) - added by westonruter 6 weeks ago.
Clean PHP_CodeSniffer complaints
41871.diff (11.2 KB) - added by ryotsun 6 weeks ago.
Full version
41871_4.diff (12.6 KB) - added by ryotsun 6 weeks ago.
Fixed codes according to the review comments
41871_5.diff (12.5 KB) - added by ryotsun 6 weeks ago.
Fixed codes according to the review comments

Download all attachments as: .zip

Change History (19)

#1 @westonruter
2 months ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

#2 @ryotsun
6 weeks ago

Although I would like to work on this ticket, I need your some assistance.
What does the test expect for?

For example,

  1. confirm its content by using assertNonEmptyMultidimensionalArray()
  2. just compared json formated string with the result of wp_json_encode().
<?php
        function test_wp_enqueue_code_editor() {
                $real_file = WP_PLUGIN_DIR . '/hello.php';
                $wp_enqueue_code_editor = wp_enqueue_code_editor( array( 'file' => $real_file ) );
                $this->assertNonEmptyMultidimensionalArray( $wp_enqueue_code_editor );

                $settings = array(
                        'codeEditor' => $wp_enqueue_code_editor,
                );
                $expected = '{"codeEditor":{"codemirror":{"indentUnit":4,"indentWithTabs":true,"inputStyle":"contenteditable","lineNumbers":true,"lineWrapping":true,"styleActiveLine":true,"continueComments":true,"extraKeys":{"Ctrl-Space":"autocomplete","Ctrl-\/":"toggleComment","Cmd-\/":"toggleComment","Alt-F":"findPersistent"},"direction":"ltr","mode":"php","autoCloseBrackets":true,"autoCloseTags":true,"matchBrackets":true,"matchTags":{"bothTags":true}},"csslint":{"errors":true,"box-model":true,"display-property-grouping":true,"duplicate-properties":true,"known-properties":true,"outline-none":true},"jshint":{"boss":true,"curly":true,"eqeqeq":true,"eqnull":true,"es3":true,"expr":true,"immed":true,"noarg":true,"nonbsp":true,"onevar":true,"quotmark":"single","trailing":true,"undef":true,"unused":true,"browser":true,"globals":{"_":false,"Backbone":false,"jQuery":false,"JSON":false,"wp":false}},"htmlhint":{"tagname-lowercase":true,"attr-lowercase":true,"attr-value-double-quotes":true,"doctype-first":false,"tag-pair":true,"spec-char-escape":true,"id-unique":true,"src-not-empty":true,"attr-no-duplication":true,"alt-require":true,"space-tab-mixed-disabled":"tab","attr-unsafe-chars":true}}}';
                $this->assertEquals( $expected, wp_json_encode( $settings ) );
        }

Are something like these okay for tests?

#3 follow-up: @westonruter
6 weeks ago

@ryotsun I'd be looking for tests on wp_enqueue_code_editor() that passing it the various args it supports (type, file, settings) then get result in the expected settings on output, as well as the expected scripts and styles being enqueued (note: you'll need to nullify $wp_scripts and $wp_styles for each test.

As for how exactly to look at the resulting $settings, I'd not suggest comparing the entire array but rather look for specific keys and values in the array. For example: https://github.com/WordPress/wordpress-develop/blob/32224a5c2a7abe7353fd16519e0686aa6603a992/tests/phpunit/tests/customize/manager.php#L2593-L2611

#4 in reply to: ↑ 3 @ryotsun
6 weeks ago

Replying to westonruter:

Thank you for your advise.
I'll work on this as soon as possible if it's all right.

@ryotsun I'd be looking for tests on wp_enqueue_code_editor() that passing it the various args it supports (type, file, settings) then get result in the expected settings on output, as well as the expected scripts and styles being enqueued (note: you'll need to nullify $wp_scripts and $wp_styles for each test.

As for how exactly to look at the resulting $settings, I'd not suggest comparing the entire array but rather look for specific keys and values in the array. For example: https://github.com/WordPress/wordpress-develop/blob/32224a5c2a7abe7353fd16519e0686aa6603a992/tests/phpunit/tests/customize/manager.php#L2593-L2611

#5 @westonruter
6 weeks ago

  • Owner set to ryotsun
  • Status changed from new to assigned

@ryotsun
6 weeks ago

#6 @ryotsun
6 weeks ago

I just wrote test only for wp_enqueue_code_editor().
And now working on the other one.

WP_Widget_Custom_HTML is a bit confusing for me how should I define test code...
Anyway, I'll resume working tomorrow.

#7 @westonruter
6 weeks ago

There are already tests for the Custom HTML widget in https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/widgets/custom-html-widget.php

Here are the new changes since the widget was introduced that tests could be added for: https://gist.github.com/westonruter/412ae02d7b87156b03dcd7068bd6440e

@westonruter
6 weeks ago

Clean PHP_CodeSniffer complaints

@ryotsun
6 weeks ago

Full version

#8 @ryotsun
6 weeks ago

Hello, I've just added all test for this ticket.
Please kindly check the attached file.
Thank you.

This ticket was mentioned in Slack in #core by ryotsun. View the logs.


6 weeks ago

#10 @westonruter
6 weeks ago

@ryotsun thank you, it's looking good.

Three more things I'd like you to add tests for:

  • When you call wp_enqueue_code_editor() add calls to wp_script_is() to actually confirm that all of the expected scripts got enqueued, like you did for the Custom HTML widget.
  • Add assertion in \Test_WP_Widget_Custom_HTML::test_enqueue_admin_scripts() to confirm that the code-editor sript gets enqueued, and thus that wp_enqueue_code_editor() was called.
  • Add tests for when a user turns off the syntax_highlighting option for their profile, to ensure that wp-codemirror and related scripts do not get enqueued.

@ryotsun
6 weeks ago

Fixed codes according to the review comments

#11 @ryotsun
6 weeks ago

@westonruter
Thank you for your review.
I fixed some test but I'm not sure it's correct. :(

Deleted test_enqueue_admin_scripts() method and added the following methods.

  • test_enqueue_admin_scripts_when_logged_in_and_syntax_highlighting_on()
  • test_enqueue_admin_scripts_when_logged_in_and_syntax_highlighting_off()

@ryotsun
6 weeks ago

Fixed codes according to the review comments

#12 @ryotsun
6 weeks ago

I uploaded the wrong one. 41871_5.diff is the correct one.

#13 @westonruter
5 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 41855:

Code Editor: Add unit tests for wp_enqueue_code_editor() and WP_Widget_Custom_HTML.

Props ryotsun.
See #12423.
Fixes #41871.

#14 @westonruter
5 weeks ago

In 41858:

Code Editor: Fix syntax error in PHP 5.2 and PHP 5.3 after [41855].

See #41871.

Note: See TracTickets for help on using tickets.