Strict get_blog_count() unit test fails on some setups

[flixos90]

The unit test Tests_Multisite_Network::test_get_blog_count_on_different_network() which was introduced in [40370] uses a very strict assertSame() call instead of assertEquals(). While the function get_blog_count() is supposed to return an integer according to its docs, the option is never cast to an integer, causing this assertion to fail.

For some reason, this never broke unit tests on Travis, and they also passed on my own setup. Several people have reported the failure on their local setup, and it appears clear that this should be fixed. While working on an unrelated patch, I just encountered the test failure on my setup as well.

Let's make this test stable by using assertEquals() instead of assertSame().

[flixos90]

In 40610:

Multisite: Fix unstable unit test for get_blog_count().

The unit test introduced in [40370] used the strict assertSame() check while comparison a count with the value of an option, which under most conditions is not an integer. While the test passed on some setups, it failed on others. This changeset ensures that assertEquals() is used to make the test stable.

Fixes #40724.

[jorbin]

It seems to me like it may make more sense to keep the assertSame and do the cast to int. The function is documented to return an integer, the expectation should be that it does.

[flixos90]

Replying to jorbin:

It seems to me like it may make more sense to keep the assertSame and do the cast to int. The function is documented to return an integer, the expectation should be that it does.

+1

I thought this wasn't something to consider because of BC, but if it's not, let's go for it!

I'd prefer to open separate tickets though, one for get_blog_count() and one for get_user_count(), which is also affected.

[jeremyfelt]

Replying to flixos90:

I thought this wasn't something to consider because of BC, but if it's not, let's go for it!

get_blog_count() has always returned a numeric string when I've tested. We're probably okay to cast this to integer so that it matches the docs/expectations, but that seems like a good change in trunk right after 4.8 rather than before.

I'd be okay to leave this ticket as is for now with the test fixed and address the rest later in a new ticket.

[flixos90]

@jeremyfelt Right, if we change the return type, it should happen in a separate ticket. I opened #40736 to consider it.