Fix cloneLooksExactlySameAsOriginal test#1421
Conversation
|
After partial revert of PR #1410, tests are failing again: https://travis-ci.org/versionpress/versionpress/builds/518248095#L1653
Date for "Test post for comments" has to be changed to preserve order on cloned site. |
"Test post for comments" is first (oldest) testing post in End2End tests and it should stay in this order also on cloned site.
|
There was an issue with the exit code from the
This is now resolved by #1422. |
|
OK, this PR is correctly broken now and tests are failing :-) |
|
This is as small as possible fix to make all tests pass again, ready for discussion and review. I started with changing |
borekb
left a comment
There was a problem hiding this comment.
Thanks, @pavelevap.
I started with changing
post_datefor some posts, but it would lead to many manual changes to enforce the right order. So I ended with removing predefinedpost_datein End2End tests and ensuring that there are no posts published at the same time.
I think this approach is practical – it feels a bit like a hack but it's better than carefully maintaining timestamps manually. So 👍.
If I searched correctly, there are 18 instances of calling the createPage method from WP-CLI workers so the test should be roughly 18 seconds slower. That is fine, given that the whole suite runs for over 10 minutes.
One thing I'm not sure about is the current "API" of the createPage method – it sounds like a generic wrapper around wp_insert_post but is actually opinionated in that that post_date typically shouldn't be provided. There are still instances of passing post_date like here or here, could they be eliminated? If so, the createPage method could then be explicit about its behavior and for example throw if $post contains post_date. That would make it safer to write tests in the future.
If we can't do that, it would still be useful to update the createPage method and its docs to mention that post_date is managed automatically and typically shouldn't be provided. Also, the 1 second sleep should be there only if post_date isn't set explicitly. Small complexities arise when a seemingly simple function is used in two different cases.
Instead of waiting everytime when post should be created, parameter waitOneSecond can be manually used only for risky situations to slow down tests.
| * @return int | ||
| */ | ||
| public function createPost(array $post) | ||
| public function createPost(array $post, $waitOneSecond = false) |
There was a problem hiding this comment.
Though it's a step in the right direction, this is still possible:
$post1["post_date"] = "2011-11-11 11:11:11";
$post2["post_date"] = "2011-11-11 11:11:11";
$wpAutomation->createPost($post1, true);
$wpAutomation->createPost($post2, true);
// date collision not really preventedI think the method could look something like this:
public function createPost(array $post, $managePostDateAutomatically = false)
{
if ($managePostDateAutomatically) {
if (isset($post["post_date"])) {
throw new Exception('post_date must not be set when managing post date automatically');
}
sleep(1);
}
// ...
}The usage of sleep becomes an implementation detail that could change in the future, the main thing is that the method guarantees certain behavior.
|
We had a discussion about this with @pavelevap and @JanVoracek yesterday and decided that the approach from #1410 is actually preferable as it runs workflow tests from a well-known state. It's true that running the clone & merge tests after End2End tests allowed us to uncover an important detail about VersionPress' behavior, documented in #1420, however, we shouldn't be using a combination of End2End and Workflow tests to verify that. So I'm going to close this PR and take a look at a change done in #1410, it will probably need some minor updates in comments but overall, that should be our solution for #1409. @pavelevap, thanks for the effort anyway, it lead us to think more deeply about how tests should be written. |
Resolves #1409
The cause of this problem depends on #1420, this PR will be only workaround to avoid posts with different titles published at the same time.