Skip to content

Instantly share code, notes, and snippets.

@costdev
Created August 12, 2022 09:19
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save costdev/bbbdcb112c397174e08b31f357f9696f to your computer and use it in GitHub Desktop.
Save costdev/bbbdcb112c397174e08b31f357f9696f to your computer and use it in GitHub Desktop.
  • The inclusion of _it_ seems unnecessary in the context of these isolated tests, UNLESS additional clarity is desired, in which case this should include the function name under test, which seems pretty common.

it is often used in JS tests/specs in the sense:

describe( 'disableBlockEditorForNavigationPostType', () => {
    // Some suites use "it"
    it( 'should only disable block editor for navigation post types',      () => {})
    // Some suites use "test"
    test( 'it should only disable block editor for navigation post types', () => {})
    // For suites using "test", removing "it" is common
    test( 'should only disable block editor for navigation post types',    () => {})
})

Converted to PHPUnit, the third example above would look like:

class Tests_Editor_DisableBlockEditorForNavigationPostType extends WP_UnitTestCase {
    public function test_should_only_disable_block_editor_for_navigation_post_types() {
    }
}

Each of these migrated tests have the same test method name, test_it_correctly_handles_different_post_types(), which seems awkward compared with others in the project. Maybe it's just me, but an alternative might be to simplify it to test_handle_different_post_types().

My issue with handle - and admittedly, I've used it myself in the past, is that it doesn't describe the expected behaviour.

i.e. test_handle_different_post_types() vs test_should_only_disable_block_editor_for_navigation_post_types().

By using test_should_ and test_should_not_ prefixes where possible, that means:

  • Raw output:
Tests_Editor_DisableBlockEditorForNavigationPostType::test_should_only_disable_block_editor_for_navigation_post_types
Tests_Editor_DisableBlockEditorForNavigationPostType::test_should_not_disable_block_editor_for_missing_post_types
  • --testdox output:
_Editor_DisableBlockEditorForNavigationPostType
 ✔ Should only disable block editor for navigation post types
 ✔ Should not disable block editor for missing post types

Theoretically, _should_ may not be necessary, but it gives a clear indication of the expected result before the reader even knows the remaining context, and we'll often internally say "Okay, this function should only / should not do something", so this saves the reader time.

@anton-vlasenko
Copy link

anton-vlasenko commented Aug 17, 2022

I've renamed the test methods.
But I didn't create test_should_not methods.
Why?
That would lead to code duplication.
The test classes would need to have the same code for test_should and test_should_not methods.
If the intention is to save time in understanding the failure, I think we are fine because these particular tests use named data sets and unique error messages.
@ironprogrammer @costdev Please review the code and let me know what you think.
WordPress/wordpress-develop#3005

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment