Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 weeks ago

#59655 closed task (blessed) (fixed)

Tests: Reduce usage of assertEquals for 6.5

Reported by: hellofromTonya Owned by:
Priority: normal Milestone: 6.5
Component: Build/Test Tools Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Follow-up to:

The assertEquals() test method does not check that the types of the expected and actual values match. This can hide subtle bugs especially when the values are falsey.

Tasks:

  • Switch to using assertSame() when the type of the value in the assertion is important
  • Replace overall usage of assertEquals() with type-strict assertion methods, with the aim of potentially removing its usage altogether

To help ease the effort of merging tests, changes should also be made upstream in the Gutenberg repository.

Change History (12)

#1 @SergeyBiryukov
3 years ago

In 57244:

Tests: Use assertSame() in some newly introduced tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [55859], [56380], [56802], [57115], [57129], [57185].

See #59655.

This ticket was mentioned in ​PR #1768 on ​WordPress/wordpress-develop by ​@costdev.


3 years ago
#2

  • Keywords has-patch has-unit-tests added

This PR replaces assertEquals() with more appropriate, stricter assertions where possible and without making any changes to source.

For easier reviewing, commits have been separated based on the replacement assertion and any additional changes required to implement the stricter assertion.

Trac ticket: https://core.trac.wordpress.org/ticket/59655
Trac ticket: https://core.trac.wordpress.org/ticket/55654

#3 @SergeyBiryukov
3 years ago

In 57650:

Tests: Use assertSame() in get_comment_pages_count() tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [27055], [48937], [54402], [57244], [57648].

Props costdev.
See #58683, #59655.

#4 @SergeyBiryukov
3 years ago

In 57653:

Tests: Use assertSame() in WP_Query tests involving ::$max_num_pages property.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [48937], [54402], [54768], [57648].

Props costdev.
See #58683, #59655.

#5 @SergeyBiryukov
3 years ago

In 57656:

Tests: Use assertSame() in WP_REST_Navigation_Fallback_Controller tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [56052].

See #59655.

#6 @SergeyBiryukov
3 years ago

In 57673:

Tests: Use assertSame() in wp_read_image_metadata() tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [49/tests], [198/tests], [34374], [48937], [52269].

Props costdev.
See #59655.

#7 @SergeyBiryukov
3 years ago

In 57680:

Tests: Use assertSame() in wp_insert_post() tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [34085], [35183], [48937], [53782], [53785], [53883], [54402].

Props costdev.
See #59655.

#8 @SergeyBiryukov
3 years ago

In 57705:

Tests: Use assertSame() in comment_exists() tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [34456], [34460].

Props costdev.
See #59655.

#9 @SergeyBiryukov
3 years ago

In 57706:

Tests: Use assertSame() in new_admin_email_subject filter test.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [57283].

See #59655.

#10 @SergeyBiryukov
3 years ago

In 57744:

Tests: Use assertSame() in post meta revisioning tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Includes correcting the test class name.

Follow-up to [56714].

See #59655.

#11 @swissspidy
3 years ago

  • Resolution → fixed
  • Status new → closed

Closing for now since 6.5 has been branched, so any new updates like this can happen in trunk for 6.6.

​@lancewillett commented on ​PR #1768:


3 weeks ago
#12

Closing as stale. The linked ticket, #63169, was closed for 6.9. The work to reduce assertEquals() usage in 7.2 is tracked on #64895, which has its own PRs. Thanks for starting this, @costdev.

Note: See TracTickets for help on using tickets.