Automattic / Co-Authors-Plus

Multiple bylines and Guest Authors for WordPress
https://wordpress.org/plugins/co-authors-plus/
GNU General Public License v2.0
291 stars 204 forks source link

Failing tests in master #589

Closed TheCrowned closed 1 year ago

TheCrowned commented 6 years ago

A good number of tests are failing in master due to these two commits:

https://github.com/Automattic/Co-Authors-Plus/pull/544/commits/8e2df320af0073cbbd74ab5b11f6f3e43daa2067 https://github.com/Automattic/Co-Authors-Plus/pull/544/commits/6509556c2b62e2832bed8ba06c33895e808650bf

screenshot from 2018-08-15 10-09-45

The issue arises from the fact that now every WP user has a linked guest author. So there are lots of tests failing because they

  1. create a WP user
  2. assert that indeed no guest author profile is linked to it
  3. link a guest author
  4. assert again.

However, the first assertion fails cause WP users get a guest author ON CREATION by default now. I am convinced that this is the correct behavior, as explained in #544, because we often run wp co-authors-plus create-guest-authors just after installing CAP, and now we have added this behavior as default on install.

There some more tests failing due to my mistake of calling cap_create_guest_authors() in tests for the search function, un-needed since they are created by default on user creation. I guess I didn't notice/got confused because it was shipped in a later commit.

My next steps would be to:

  1. remove all calls to cap_create_guest_authors(), but add one at the top of Test_CoAuthors_Plus::setUp() because we need to create guest author for admin (another option would be to create guest author by ID with ID being 1);
  2. remove all calls to create_guest_author_from_user_id() and all assertion that check whether a guest author does NOT have a linked account, since they inevitably fail.
philipjohn commented 6 years ago

I actually reverted this earlier (#590) when looking into another PR where tests were failing. Thanks for digging out the detail. I'm going to assign this issue to @TheCrowned as the original author of #544 .

TheCrowned commented 6 years ago

@philipjohn will take care of it in the next few hours - what should I do, commit to the old branch of #544 even if it's merged, or a new one? Thanks

GaryJones commented 1 year ago

Tests all currently pass, so I'll close this out.