DateThemeTestCase randomly(?) fails on testDateDisplayCombination

[quicksketch]

Description of the bug
All PRs are currently failing on DateThemeTestCase test. Looks like the expected output isn't matching in the test.

Steps To Reproduce
Run the test suite, in particular DateThemeTestCase:

./core/scripts/run-tests.sh DateThemeTestCase --verbose --url http://backdrop.local --force

Actual behavior

Date: Date Theme (DateThemeTestCase) 4 passes, 1 fail, 0 exceptions, and 1 debug message

Expected behavior
All tests should be passing.

Additional information
Add any other information that could help, such as:

• Backdrop CMS version: 1.13.0

---

PR by @indigoxela: backdrop/backdrop#2689

[quicksketch]

Well, now I'm not seeing the same failure. I saw it both on Zen.CI and on my local, but rerunning the tests everything is working fine now. This may be a race condition or time-specific

[quicksketch]

On my local, here are the expected and actual results:

Expected

<span class="date-display-single">24.05.2019 - <span class="date-display-range"><span class="date-display-start">23:52</span> to <span class="date-display-end">00:52 Europe/Moscow</span></span> </span><div class="date-display-remaining"><span class="date-display-remaining">To event remaining 7 days</span></div>

Actual

<span class="date-display-range"><span class="date-display-start">24.05.2019 - 23:52 </span> to <span class="date-display-end">25.05.2019 - 00:52  Europe/Moscow</span></span><div class="date-display-remaining"><span class="date-display-remaining">To event remaining 7 days</span></div>

[indigoxela]

Trying to reproduce here, but wasn't able to. Tests are passing on both, php7.2 and php5.6.

But obviously there's something wrong with "testDateDisplayCombination". I'll have a look.

[indigoxela]

Got it. It was a bad assumption to think, "+1 week" would be on the same day automatically. The test must fail depending on current time.
I'll create a new pull request as soon as I found a cleaner way to choose a date/time.

[indigoxela]

I've just created a pull request, which fixes the problem with failing tests depending on testing time.

Sorry for the mess.

[herbdool]

I've reviewed the code. I'm not sure at what time to check if test will fail, when did it fail before.

[indigoxela]

I'm not sure at what time to check if test will fail

Yes, that's tricky.

The check for "remaining days" requires dynamic dates and makes this test complex - maybe too complex.
We should probably simplify it. Otherwise it's too hard to fix it in case it breaks (again).

But how to simplify it, but still keep it meaningful?

[indigoxela]

I updated the test.

When should the previous test version fail:

The problem with my test was that if local time in the timezone (Europe/Moscow) I chose for testing was between 23:01 and 23:59, the test failed because the second time (+ one hour) slipped over to the next day.

Europe/Moscow is UTC + 3 hours, hence the test failed between 20:01 and 20:59 UTC.
The suitable local time for reproducing the problem really depends on the timezone you're (or the testing server is) in.
Assuming it is somewhere around Los Angeles (PDT), this would be 13:01 to 13:59.

[quicksketch]

The check for "remaining days" requires dynamic dates and makes this test complex - maybe too complex.
We should probably simplify it. Otherwise it's too hard to fix it in case it breaks (again).

I don't think we need to test dynamic dates. We should pick a set of expected edge-cases, like the end of the day/month/year and then test those. If we find a bug later, we should add the case that causes it, that way the tests will always pass (or fail) every time.

[indigoxela]

If we find a bug later...

There already was a bug in handling remaining days in date theme. That's why I decided to test it.

Up to and including release 1.12.6 a date format like e: Y-m-d H:i O
ended up looking like:

Europe/Vienna: 2019-05-30 12:00 to 13:00 Europe/Vienna +0200
To event remaining 6 days
+0200 Europe/Vienna +0200

Remaining days somewhere in the middle.

Since this is fixed, it correctly shows as:

Europe/Vienna: 2019-05-30 12:00 to 13:00 +0200
To event remaining 6 days

Remaining days only show up if the date is in future, hence this testing date needs to be dynamic.
On the other hand I'd understand, if this test was skipped or simplified (testing without remaining days with a fixed date) to make it easier to understand and fix.

[quicksketch]

Remaining days only show up if the date is in future, hence this testing date needs to be dynamic.

Ah I see! Sorry I did not think that through. Makes sense. 👍

[quicksketch]

Merged backdrop/backdrop#2689 into 1.x and 1.13.x. Thanks @indigoxela for the initial fix and then for being so responsible as to clean up these tests!

[klonos]

This might have not fixed things completely: backdrop/backdrop#2729

[klonos]

In #4180 we've merged an initial PR to fix the random time failures, but I kept getting them, and also seeing the same random timezone-related failures in PRs filed by others. I have now re-opened #4180, and filed a follow-up PR; so let's close this one here.