Problem/Motivation

`DateFormatter()` assumes 30 days per month, while February only has 28 days. Causes fails in tests.

Proposed resolution

Adjust `TimestampAgoFormatterTest()` to test the spirit of what is being covered, but make not glitchy because calendar systems are terrible.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 interdiff-02-04.txt1.19 KBmpdonadio
#5 3035318-04.patch2.37 KBmpdonadio
#2 3035318-02.patch1.18 KBmpdonadio
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpdonadio created an issue. See original summary.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Status: Active » Needs review
FileSize
3035318-02.patch1.18 KB

There is no reason to set the granularity to three here. That is not the purpose of the test, which is just to make sure the sample format appears correctly. `TimestampFormatterTest` covers the ago logic.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Title: `DateFormatter` assumes 30 days per month, while February only has 28 days. Causes fails in tests. » `DateFormatter()` assumes 30 days per month, while February only has 28 days. Causes fails in tests.
Assigned: mpdonadio » Unassigned
Issue summary: View changes

Long term, I wonder if we should think about using `\DateInterval()` instead of a home-grown algorithm. I don't recall why we didn't do that in the first place.

And this is currently a Critical because HEAD is failing, and probably will for the next few days.

Status: Needs review » Needs work

The last submitted patch, 2: 3035318-02.patch, failed testing. View results

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Status: Needs work » Needs review
FileSize
3035318-04.patch2.37 KB
interdiff-02-04.txt1.19 KB
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Reviewed & tested by the community

Was wondering if we should have a follow-up to test the granularity again, but per #2, that is already covered in another test.

Krzysztof Domański’s picture

Krzysztof Domański
Location Poland
CreditAttribution: Krzysztof Domański at abventor commented
Issue tags: +Random test failure

Each test ends with a failure now.

https://www.drupal.org/pift-ci-job/1210027 // probably first time
https://www.drupal.org/pift-ci-job/1210029
https://www.drupal.org/pift-ci-job/1210030
https://www.drupal.org/pift-ci-job/1210035
https://www.drupal.org/pift-ci-job/1210074
https://www.drupal.org/pift-ci-job/1210075
https://www.drupal.org/pift-ci-job/1209483

Krzysztof Domański’s picture

Krzysztof Domański
Location Poland
CreditAttribution: Krzysztof Domański at abventor commented
Issue tags: -Random test failure
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed bdfafab7fb to 8.7.x and 6f3f6fb3ba to 8.6.x. Thanks!

  • alexpott committed bdfafab on 8.7.x 
    Issue #3035318 by mpdonadio: `DateFormatter()` assumes 30 days per month...

  • alexpott committed 6f3f6fb on 8.6.x 
    Issue #3035318 by mpdonadio: `DateFormatter()` assumes 30 days per month...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.