Speed up unit tests by disabling password hashing

[Frank Klein]

Whenever the factory creates a new user during a test, it calls wp_insert_user(). This function calls wp_hash_password(), which internally uses PasswordHash::HashPassword() to create a hash of the default password.

In the context of most unit tests, users do not need hashed passwords. Hashing has a performance impact, so the tests should run a bit faster if we avoid hashing if we don't need to.

The attached patch introduces a mock password hasher, that is used by the entirety of the tests The exception are a few tests that rely on authentication to work properly.

[johnbillion]

Some background: The PasswordHash class used for password hashing calls md5() repeatedly to generate a portable hash. This accounts for millions of calls to md5() and by far the greatest number of calls to a single function when running the full test suite.

That said, the combined wall time of md5() is not that high, taking up around 3% of wall time from my testing. 42904.patch​ works and the tests pass, but the time saving unfortunately isn't as great as I hoped it would be. The time saving on my local machine is around 15 seconds reduction on a test suite run that takes 1m 40s.

[boonebgorges]

15 seconds reduction off of 100s total is a pretty big win, IMO. I don't see the downside.

One thing I'd suggest is that the $GLOBALS juggling could happen inside of the User factory class. Perhaps the create() method could accept a skip_password_hash param that would default to false. This would make it more transparent how to override when a test case needs a hashed pass.

[Frank Klein]

Thanks for the feedback @boonebgorges, I adapted the patch as proposed.

I also added a unit test for this functionality of the user fixtures factory. It is excluded by default, as I don't see the need to test the factory every time the suite runs.

I did not add the new argument to the inline documentation of the user fixtures factory, as it is not a straightforward change to make. Something for another ticket.

[pento]

42904-2.patch​ looks good. It needs an @since bump, but is otherwise fine to commit.

[pento]

Actually, I just got back to this ticket. It seems that WP_UnitTest_Factory_For_User::create_object() is only allowing it for tests that pass skip_password_hash, but currently no tests are doing that. Disabling that check gives the speed boost, but causes a bunch of errors.

So, I like the direction of 42904-2.patch​, but it needs to be updated to turn off password hashing on the tests that don't need it.

[desrosj]

@frank-klein are you able to refresh this based on the feedback above? 5.2 beta 1 is in less than 2 days, but this is a test related change, so it could land during the beta. Leaving it in the milestone for now.

[desrosj]

Punting this, but it can be moved back if ready for 5.2.

[desrosj]

This still needs a refresh and more work. With 5.3 beta 1 in 3 days, I'm going to punt this.