Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT Fix unit tests #11409

Merged
merged 1 commit into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions tests/php/Forms/DateFieldDisabledTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ class DateFieldDisabledTest extends SapphireTest
protected function setUp(): void
{
parent::setUp();
i18n::set_locale('en_NZ');
// Set to an explicit locale so project-level locale swapping doesn't affect tests
i18n::set_locale('en_US');
DBDatetime::set_mock_now('2011-02-01 8:34:00');
}

Expand All @@ -22,7 +23,7 @@ public function testFieldToday()
$actual = DateField_Disabled::create('Test')
->setValue('2011-02-01')
->Field();
$expected = '<span class="readonly" id="Test">1/02/2011 (today)</span>';
$expected = '<span class="readonly" id="Test">Feb 1, 2011 (today)</span>';
$this->assertEquals($expected, $actual);

// Test today's date with time
Expand All @@ -38,14 +39,14 @@ public function testFieldWithDifferentDay()
$actual = DateField_Disabled::create('Test')
->setValue('2011-01-27')
->Field();
$expected = '<span class="readonly" id="Test">27/01/2011, 5 days ago</span>';
$expected = '<span class="readonly" id="Test">Jan 27, 2011, 5 days ago</span>';
$this->assertEquals($expected, $actual);

// Test future
$actual = DateField_Disabled::create('Test')
->setValue('2011-02-06')
->Field();
$expected = '<span class="readonly" id="Test">6/02/2011, in 5 days</span>';
$expected = '<span class="readonly" id="Test">Feb 6, 2011, in 5 days</span>';
$this->assertEquals($expected, $actual);
}

Expand Down
11 changes: 6 additions & 5 deletions tests/php/Forms/DatetimeFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class DatetimeFieldTest extends SapphireTest
protected function setUp(): void
{
parent::setUp();
i18n::set_locale('en_NZ');
// Set to an explicit locale so project-level locale swapping doesn't affect tests
i18n::set_locale('en_US');
// Fix now to prevent race conditions
DBDatetime::set_mock_now('2010-04-04');
$this->timezone = date_default_timezone_get();
Expand Down Expand Up @@ -141,14 +142,14 @@ public function testSetValueWithLocalised()

$datetimeField
->setHTML5(false)
->setLocale('en_NZ');
->setLocale('de_DE');
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

$datetimeField->setSubmittedValue('29/03/2003 11:00:00 pm');
$this->assertEquals($datetimeField->dataValue(), '2003-03-29 23:00:00');
$datetimeField->setSubmittedValue('29/03/2003 23:00:00');
$this->assertEquals('2003-03-29 23:00:00', $datetimeField->dataValue());

// Some localisation packages exclude the ',' in default medium format
$this->assertMatchesRegularExpression(
'#29/03/2003(,)? 11:00:00 (PM|pm)#',
'#29.03.2003(,)? 23:00:00#',
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
$datetimeField->Value(),
'User value is formatted, and in user timezone'
);
Expand Down
35 changes: 18 additions & 17 deletions tests/php/ORM/DBDateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ protected function setUp(): void
$this->oldError = error_reporting();
// Validate setup
assert(date_default_timezone_get() === 'UTC');
i18n::set_locale('en_NZ');
// Set to an explicit locale so project-level locale swapping doesn't affect tests
i18n::set_locale('en_US');
}

protected function tearDown(): void
Expand Down Expand Up @@ -48,42 +49,42 @@ protected function restoreNotices()
public function testNiceDate()
{
$this->assertEquals(
'31/03/2008',
'Mar 31, 2008',
Comment on lines -51 to +52
Copy link
Member Author

@GuySartorelli GuySartorelli Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old date is effective the same as you'd get when using IntlDateFormatter::SHORT, though we're using IntlDateFormatter::MEDIUM here.

Steve and I had talked in person about making the default configurable.
That would be doable for DateField_Disabled in the above test, because the default for that is returned from DateField::getDateLength() as a fallback value. Though its notable that none of the tests for DateField itself failed... so I'm not sure about the value of making that change but can do if the reviewer wants me to.

That's not possible for DBDate in this test though, because the default value is a default argument in the method signature of DBDate::getFormatter() and we can't change method signatures in patches or minors.

Copy link
Member

@emteknetnz emteknetnz Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though its notable that none of the tests for DateField itself failed

Looks like that's because only the internal values were being validated in DateFieldTest, rather than the formatted values like Nice()

so I'm not sure about the value of making that change but can do if the reviewer wants me to.

Reading php/php-src#16127 (comment) particularly around the "no contract" bit, I guess it's correct to not have config here and not attempt to retain backwards compatibility. Simply updating our unit tests is the correct to test the new format is the correct response I think.

DBField::create_field('Date', 1206968400)->Nice(),
"Date->Nice() works with timestamp integers"
);
$this->assertEquals(
'30/03/2008',
'Mar 30, 2008',
DBField::create_field('Date', 1206882000)->Nice(),
"Date->Nice() works with timestamp integers"
);
$this->assertEquals(
'31/03/2008',
'Mar 31, 2008',
DBField::create_field('Date', '1206968400')->Nice(),
"Date->Nice() works with timestamp strings"
);
$this->assertEquals(
'30/03/2008',
'Mar 30, 2008',
DBField::create_field('Date', '1206882000')->Nice(),
"Date->Nice() works with timestamp strings"
);
$this->assertEquals(
'4/03/2003',
'Mar 4, 2003',
DBField::create_field('Date', '4.3.2003')->Nice(),
"Date->Nice() works with D.M.YYYY format"
);
$this->assertEquals(
'4/03/2003',
'Mar 4, 2003',
DBField::create_field('Date', '04.03.2003')->Nice(),
"Date->Nice() works with DD.MM.YYYY format"
);
$this->assertEquals(
'4/03/2003',
'Mar 4, 2003',
DBField::create_field('Date', '2003-3-4')->Nice(),
"Date->Nice() works with YYYY-M-D format"
);
$this->assertEquals(
'4/03/2003',
'Mar 4, 2003',
DBField::create_field('Date', '2003-03-04')->Nice(),
"Date->Nice() works with YYYY-MM-DD format"
);
Expand All @@ -107,7 +108,7 @@ public function testInvertedYearCorrection()
{
// iso8601 expects year first, but support year last
$this->assertEquals(
'4/03/2003',
'Mar 4, 2003',
DBField::create_field('Date', '04-03-2003')->Nice(),
"Date->Nice() works with DD-MM-YYYY format"
);
Expand Down Expand Up @@ -152,32 +153,32 @@ public function testShortMonth()
public function testLongDate()
{
$this->assertEquals(
'31 March 2008',
'March 31, 2008',
DBField::create_field('Date', 1206968400)->Long(),
"Date->Long() works with numeric timestamp"
);
$this->assertEquals(
'31 March 2008',
'March 31, 2008',
DBField::create_field('Date', '1206968400')->Long(),
"Date->Long() works with string timestamp"
);
$this->assertEquals(
'30 March 2008',
'March 30, 2008',
DBField::create_field('Date', 1206882000)->Long(),
"Date->Long() works with numeric timestamp"
);
$this->assertEquals(
'30 March 2008',
'March 30, 2008',
DBField::create_field('Date', '1206882000')->Long(),
"Date->Long() works with numeric timestamp"
);
$this->assertEquals(
'3 April 2003',
'April 3, 2003',
DBField::create_field('Date', '2003-4-3')->Long(),
"Date->Long() works with YYYY-M-D"
);
$this->assertEquals(
'3 April 2003',
'April 3, 2003',
DBField::create_field('Date', '3.4.2003')->Long(),
"Date->Long() works with D.M.YYYY"
);
Expand All @@ -186,7 +187,7 @@ public function testLongDate()
public function testFull()
{
$this->assertEquals(
'Monday, 31 March 2008',
'Monday, March 31, 2008',
DBField::create_field('Date', 1206968400)->Full(),
"Date->Full() works with timestamp integers"
);
Expand Down
13 changes: 7 additions & 6 deletions tests/php/ORM/DBDatetimeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class DBDatetimeTest extends SapphireTest
protected function setUp(): void
{
parent::setUp();
i18n::set_locale('en_NZ');
// Set to an explicit locale so project-level locale swapping doesn't affect tests
i18n::set_locale('en_US');
}

public function testNowWithSystemDate()
Expand Down Expand Up @@ -126,23 +127,23 @@ public function testNice()
$date = DBDatetime::create_field('Datetime', '2001-12-11 22:10:59');

// note: Some localisation packages exclude the ',' in default medium format
i18n::set_locale('en_NZ');
$this->assertMatchesRegularExpression('#11/12/2001(,)? 10:10 PM#i', $date->Nice());
i18n::set_locale('de_DE');
$this->assertMatchesRegularExpression('#11.12.2001(,)? 22:10#i', $date->Nice());

i18n::set_locale('en_US');
$this->assertMatchesRegularExpression('#Dec 11(,)? 2001(,)? 10:10 PM#i', $date->Nice());
$this->assertMatchesRegularExpression('#Dec 11(,)? 2001(,)? 10:10\hPM#iu', $date->Nice());
}

public function testDate()
{
$date = DBDatetime::create_field('Datetime', '2001-12-31 22:10:59');
$this->assertEquals('31/12/2001', $date->Date());
$this->assertEquals('Dec 31, 2001', $date->Date());
}

public function testTime()
{
$date = DBDatetime::create_field('Datetime', '2001-12-31 22:10:59');
$this->assertMatchesRegularExpression('#10:10:59 PM#i', $date->Time());
$this->assertMatchesRegularExpression('#10:10:59\hPM#iu', $date->Time());
}

public function testTime24()
Expand Down
4 changes: 2 additions & 2 deletions tests/php/ORM/DBTimeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ public function testParse($input, $expected)
public function testNice()
{
$time = DBTime::create_field('Time', '17:15:55');
$this->assertMatchesRegularExpression('#5:15:55 PM#i', $time->Nice());
$this->assertMatchesRegularExpression('#5:15:55\hPM#iu', $time->Nice());
}

public function testShort()
{
$time = DBTime::create_field('Time', '17:15:55');
$this->assertMatchesRegularExpression('#5:15 PM#i', $time->Short());
$this->assertMatchesRegularExpression('#5:15\hPM#iu', $time->Short());
}
}
Loading