WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#30080 closed task (blessed) (fixed)

Consolidate, split, and organize current tests in ms-site group

Reported by: jeremyfelt Owned by: jeremyfelt
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords:
Focuses: multisite Cc:
PR Number:

Description

There are a couple areas in tests/multisite/site.php where we are redundant or organized in quirky ways that make it tough to follow what exactly is being tested.

I'd like to go through and find areas of duplicity and generally improve the layout of tests here.

Related: #29896, #30017

Attachments (3)

30080.diff (6.5 KB) - added by jeremyfelt 5 years ago.
30080-wmpu_delete_blog.diff (7.0 KB) - added by jeremyfelt 5 years ago.
30080-quota-tests.diff (8.2 KB) - added by jeremyfelt 5 years ago.

Download all attachments as: .zip

Change History (25)

@jeremyfelt
5 years ago

#1 @jeremyfelt
5 years ago

In 30080.diff:

  • Split test_create_and_delete_blog() into test_created_site_details() and test_wpmu_delete_blog().
  • test_created_site_details() should only need one site.
  • test_wpmu_delete_blog() requires two sites
  • Remove test_getters() as the testing of everything except for get_blog_address_by_name() is tested elsewhere.
  • Move the test for get_blog_address_by_name() to test_created_site_details()

#2 @jeremyfelt
5 years ago

In 30006:

Begin cleanup of ms-sites group unit tests

  • Split test_create_and_delete_blog() into two tests.
  • Clean up number of sites created during tests.
  • Remove test_getters(), move get_blog_address_by_name() test to test_created_site_details()

See #30080

#3 @jeremyfelt
5 years ago

In 30007:

Improve tests for get_blog_id_from_url()

Expand tests to cover additional cache and lookup scenarios. Explicitly test the reaction of get_blog_id_from_url() when $drop = false is passed to wpmu_delete_blog(). See #30080

Fixes #30088

#4 @jeremyfelt
5 years ago

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

#5 @jeremyfelt
5 years ago

  • Summary changed from Consolidate, split, and organize current tests in ms-sites group to Consolidate, split, and organize current tests in ms-site group

In testing wpmu_delete_blog, we can cover several various scenarios: (so far)

  • Test data in cache after a site is deleted with drop set to false.
  • Test data in tables after a site is deleted with drop set to false.
  • Test data in cache after a site is deleted with drop set to true.
  • Test data in tables after a site is deleted with drop set to true.
  • Test data in cache after the main site is deleted with drop set to true.
  • Test data in tables after the main site is deleted with drop set to true.
  • Test existence of upload directories after a site is deleted. See #30121

#6 @jeremyfelt
5 years ago

30080-wmpu_delete_blog.diff, as misspelled as it is, covers the tests for everything in the above comment except for the existence of upload directories.

This starts heading down the road of explicit test names. Feedback is welcome!

#7 @jeremyfelt
5 years ago

In 30106:

Expand tests around wpmu_delete_blog()

  • Test cache after a site is deleted or flagged as deleted.
  • Test tables after a site is deleted or flagged as deleted.
  • Test tables and cache after the main site is deleted.
  • Test site count after a site is deleted or flagged as deleted.

See #30080

#8 @jeremyfelt
5 years ago

In 30151:

Expand and clarify tests for get_blog_post()

Remove unnecessary user factory use. Be explicit about test scenarios.

See #30080

#9 @jeremyfelt
5 years ago

In 30152:

Split tests for is_main_site()

Break multiple assertions from one method into multiple methods with one assertion each.

See #30080

#10 @jeremyfelt
5 years ago

In 30174:

Split tests for wp_get_sites()

Test various arguments for wp_get_sites() in a more intentful way rather than in one large test. Leave tests for limit and offset together for convenience.

See #30080

#11 @jeremyfelt
5 years ago

In 30175:

Improve layout of test for cached invalid, then valid site details

  • Rename method to test_get_blog_details_when_site_does_not_exist().
  • Always assume MAX(blog_id) is 1 and therefore always create a burner.
  • Remove tests specific to wpmu_delete_blog() and cache, to be handled elsewhere.
  • Remove extra asertions.

See #30080, #23405

#12 @jeremyfelt
5 years ago

  • Type changed from enhancement to task (blessed)

Not too much more to go on this ticket. I'll be hitting it pretty hard this weekend.

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


5 years ago

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


5 years ago

#15 @jeremyfelt
5 years ago

In 30715:

Split and improve multisite tests for upload quota

Break a single test with many assertions into many tests with single assertions.

In the process, provide separate and comprehensive tests for upload_is_user_over_quota(), is_upload_space_available(), and get_space_allowed().

Also removes a check for BLOGSUPLOADDIR, a constant that never existed. New tests will need to be introduced to handle the ms-files group.

See #30080

#16 @jeremyfelt
5 years ago

In 30719:

Split tests for get_blog_id_from_url()

Breaks the many assertions for get_blog_id_from_url() into individual tests.

See #30080

#17 follow-up: @DrewAPicture
5 years ago

@jeremyfelt: Anything left you wanted to do here for 4.1 or should we just move this to Future Release to get it off the milestone?

#18 @jeremyfelt
5 years ago

In 30784:

Split current tests for update_blog_details()

The current tests for upload_blog_details() were focused on the actions fired whenever a site is marked as spam, archived, deleted, or matured. This breaks those into individual sections with fewer assertions per test.

See #30080

#19 in reply to: ↑ 17 @jeremyfelt
5 years ago

Replying to DrewAPicture:

@jeremyfelt: Anything left you wanted to do here for 4.1 or should we just move this to Future Release to get it off the milestone?

Almost there. I'll verify and close it out tonight.

#20 @jeremyfelt
5 years ago

In 30785:

Split current tests for update_blog_status()

The current tests for update_blog_status() mirrored the tests for update_blog_details() in many ways and can be split in the same way. A noticeable difference is that the the matching actions fire even when no change is made to a field.

See #30080

#21 @jeremyfelt
5 years ago

In 30786:

Clean up factory arguments in ms-sites group.

  • Arguments for user, path, and title are only necessary when we need to do something with those arguments later. Most cases in the ms-sites group do not require them.
  • In test_get_blog_id_from_url_is_case_insensitive(), we should pass a lowercase domain argument.
  • A user factory in test_switch_restore_blog() is not necessary.

See #30080

#22 @jeremyfelt
5 years ago

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

We're good here. In the future it may be nice to revisit a couple others, specifically test_get_blog_details(). There are likely other site related functions that are not represented.

Note: See TracTickets for help on using tickets.