Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#47195 closed defect (bug) (fixed)

Switch WP_UnitTest_Factory_For_Blog->create_object() to use wp_insert_site()

Reported by: danielbachhuber's profile danielbachhuber Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

WP_UnitTest_Factory_For_Blog->create_object() currently uses wpmu_create_blog(), which produces a deprecated notice (and then fails the test).

It should be switched over to use the wp_insert_site() that landed in r43548

Attachments (2)

47195.diff (891 bytes) - added by davidbaumwald 6 years ago.
Patch
47195.2.diff (1.7 KB) - added by davidbaumwald 6 years ago.
Updated patch

Download all attachments as: .zip

Change History (14)

#1 follow-up: @jeremyfelt
6 years ago

+1, very much.

One note: wpmu_create_blog() itself should not produce a deprecated notice. We left that off for the near term because of how widely used it is. The wpmu_new_blog hook from that function was moved over to wp_insert_site() and that does have a notice attached to it. There's a chance that this would still cause test issues after moving the factory to wp_insert_site().

@davidbaumwald
6 years ago

Patch

#2 @SergeyBiryukov
6 years ago

  • Keywords has-patch added; needs-patch removed

#3 in reply to: ↑ 1 @danielbachhuber
6 years ago

Replying to jeremyfelt:

One note: wpmu_create_blog() itself should not produce a deprecated notice. We left that off for the near term because of how widely used it is. The wpmu_new_blog hook from that function was moved over to wp_insert_site() and that does have a notice attached to it. There's a chance that this would still cause test issues after moving the factory to wp_insert_site().

Yep. My test still failed after I changed it over to wp_insert_site(), which made me realize I had some code hooked on to wpmu_new_blog too. It was the hook that was causing the deprecation notice.

#4 follow-up: @jeremyfelt
6 years ago

Thanks for the patch, @davidbaumwald. I think we may need to do a bit more in the $args that are passed to wp_insert_site(). I haven't looked closely yet, but I get 11 failures when running the multisite tests after 47195.diff is applied.

#5 in reply to: ↑ 4 @davidbaumwald
6 years ago

Replying to jeremyfelt:

Thanks for the patch, @davidbaumwald. I think we may need to do a bit more in the $args that are passed to wp_insert_site(). I haven't looked closely yet, but I get 11 failures when running the multisite tests after 47195.diff is applied.

Thanks for the feedback @jeremyfelt. I've added to the factory some code that's similar in effect to what wpmu_create_blog was doing. This has resolved the test failures for me. Let me know if there are any issues remaining with the patch or if you'd like to see a different approach.

@davidbaumwald
6 years ago

Updated patch

#6 @netweb
5 years ago

  • Milestone changed from 5.3 to 5.4

Due to lack of updates, punting this to 5.4 to clear the way for 5.3

#7 @SergeyBiryukov
5 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#8 @SergeyBiryukov
5 years ago

In 47011:

Tests: Switch WP_UnitTest_Factory_For_Blog::create_object() to use wp_insert_site().

Map some arguments for backward compatibility with wpmu_create_blog() previously used there.

Props davidbaumwald, danielbachhuber, jeremyfelt, SergeyBiryukov.
See #47195.

#9 @SergeyBiryukov
5 years ago

In 47012:

Tests: Replace most instances of wpmu_delete_blog() not specifically testing that function with wp_delete_site().

Follow-up to [47011].

See #47195.

#10 @SergeyBiryukov
5 years ago

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

In 47013:

Tests: Update legacy arguments passed to WP_UnitTest_Factory_For_Blog::create_object().

This converts the arguments originally meant for wpmu_create_blog() to the ones used by wp_insert_site().

Follow-up to [47011].

Fixes #47195.

#11 @SergeyBiryukov
5 years ago

In 47014:

Tests: Set network_id instead of site_id in WP_UnitTest_Factory_For_Blog defaults.

Follow-up to [47011], [47013].

See #47195.

#12 @SergeyBiryukov
5 years ago

For future reference, the site_id and meta mapping added to WP_UnitTest_Factory_For_Blog in [47011] can now be removed, and the tests should still pass.

That said, I think it makes sense to keep the mapping for now for back compat just in case there are any plugins using the factory in their own tests.

Note: See TracTickets for help on using tickets.