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.

Sign up for GitHub

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

Fix testUsernameChangeCostWindow setup #12062

Merged
merged 5 commits into from
Mar 31, 2025
Merged

Fix testUsernameChangeCostWindow setup #12062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d97d8e
fix test setup
notbakaneko Mar 31, 2025
848b7eb
use NoOverflow versions to get dates on the right side of month bound…
notbakaneko Mar 31, 2025
9fbfe1e
addYearsNoOverflow for all
notbakaneko Mar 31, 2025
3ba1cbe
time travel with same base
notbakaneko Mar 31, 2025
26f93ba
Merge branch 'master' into fix-username-cost-window-test
nanaya Mar 31, 2025
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
2 changes: 1 addition & 1 deletion app/Models/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public function usernameChangeCost()
$tier = max(
$this->usernameChangeHistory()
->paid()
->where('timestamp', '>', Carbon::now()->subYears(3))
->where('timestamp', '>', Carbon::now()->subYearsNoOverflow(3))
->count(),
$minTier,
);
Expand Down
22 changes: 12 additions & 10 deletions tests/Models/UserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public function testResetSessions(): void
public function testUsernameAvailableAtForDefaultGroup()
{
config_set('osu.user.allowed_rename_groups', ['default']);
$allowedAtUpTo = now()->addYears(5);
$allowedAtUpTo = now()->addYearsNoOverflow(5);
$user = User::factory()->withGroup('default')->create();

$this->assertLessThanOrEqual($allowedAtUpTo, $user->getUsernameAvailableAt());
Expand All @@ -154,7 +154,7 @@ public function testUsernameAvailableAtForDefaultGroup()
public function testUsernameAvailableAtForNonDefaultGroup()
{
config_set('osu.user.allowed_rename_groups', ['default']);
$allowedAt = now()->addYears(10);
$allowedAt = now()->addYearsNoOverflow(10);
$user = User::factory()->withGroup('gmt')->create(['group_id' => app('groups')->byIdentifier('default')]);

$this->assertGreaterThanOrEqual($allowedAt, $user->getUsernameAvailableAt());
Expand Down Expand Up @@ -185,11 +185,11 @@ public function testUsernameChangeCostMultiple()
]);

// 1 change in last 3 years
$this->travelTo(CarbonImmutable::now()->addYears(3));
$this->travelTo(CarbonImmutable::now()->addYearsNoOverflow(3));
$this->assertSame(8, $user->usernameChangeCost());

// 0 changes in last 3 years
$this->travelTo(CarbonImmutable::now()->addYears(1));
$this->travelTo(CarbonImmutable::now()->addYearsNoOverflow(1));
$this->assertSame(8, $user->usernameChangeCost());

$user->usernameChangeHistory()->create([
Expand All @@ -211,10 +211,10 @@ public function testUsernameChangeCostMultiple()
$this->assertSame(16, $user->usernameChangeCost());

// 1 changes in last 3 years
$this->travelTo(CarbonImmutable::now()->addYears(3));
$this->travelTo(CarbonImmutable::now()->addYearsNoOverflow(3));
$this->assertSame(8, $user->usernameChangeCost());
// 0 changes in last 3 years
$this->travelTo(CarbonImmutable::now()->addYears(1));
$this->travelTo(CarbonImmutable::now()->addYearsNoOverflow(1));
$this->assertSame(8, $user->usernameChangeCost());
}

Expand All @@ -236,20 +236,22 @@ public function testUsernameChangeCostType(string $type, int $cost)
public function testUsernameChangeCostWindow(int $years, int $cost)
{
$now = CarbonImmutable::now();
$this->travelTo(CarbonImmutable::now()->subYears(3));
$this->travelTo($now->subYearsNoOverflow(3));

$user = User::factory()->create();
while (CarbonImmutable::now()->isBefore($now)) {
// every 6 months for 3 years = 6
// using isBefore to setup adds too many when run at month boundaries.
for ($i = 0; $i < 6; $i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

although it's not for another three years, there's a vaguely similar problem when run on feb 29

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it ironically works fine because usernameChangeCost doesn't check if the date is in the future

Copy link
Collaborator

@nanaya nanaya Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

still failing with $now = CarbonImmutable::now()->subYears(1)->startOfMonth()->subDays(1);...

I guess it's more like something just doesn't use the carbon date...?

Nope, it's just $now isn't actually quite used everywhere relevant

$user->usernameChangeHistory()->create([
'timestamp' => CarbonImmutable::now(),
'type' => 'paid',
'username' => 'marty',
]);

$this->travelTo(CarbonImmutable::now()->addMonths(6));
$this->travelTo(CarbonImmutable::now()->addMonthsNoOverflow(6));
}

$this->travelTo($now->addYears($years));
$this->travelTo($now->addYearsNoOverflow($years));
$this->assertSame($cost, $user->usernameChangeCost());
}

Expand Down