Skip to content

Conversation

@zigzagdev
Copy link
Contributor

@zigzagdev zigzagdev commented Jun 24, 2025

Description

This pull request refactors the test suite to eliminate redundant model instantiations. Several test cases repeatedly constructed identical User, Item, and Book model objects inline, leading to code duplication and reduced maintainability.

To address this:

  • A set of factory-like helper methods has been introduced to create consistent test data.
  • All repeated inline instantiations were replaced by these shared methods.
  • This improves readability, enforces consistency, and simplifies future updates.

This is a non-functional change (refactor only) and does not alter the behaviour or scope of any existing tests.

Checklist

  • Add tests and ensure they pass
スクリーンショット 2025-06-24 10 38 00

@zigzagdev zigzagdev requested a review from a team as a code owner June 24, 2025 01:37
@zigzagdev zigzagdev requested a review from paulinevos June 24, 2025 01:37
@zigzagdev zigzagdev changed the title **refactor: consolidate repetitive model instantiations across tests** refactor: consolidate repetitive model instantiations across tests Jun 24, 2025
@zigzagdev zigzagdev requested a review from GromNaN June 25, 2025 09:12
@GromNaN GromNaN changed the title refactor: consolidate repetitive model instantiations across tests Consolidate repetitive model instantiations across tests Dec 8, 2025
Copilot AI review requested due to automatic review settings December 8, 2025 17:45
@GromNaN GromNaN force-pushed the feature/refactor-model-test branch from 6ecd527 to aa3b3dc Compare December 8, 2025 17:45
@GromNaN GromNaN enabled auto-merge (squash) December 8, 2025 17:45
@GromNaN GromNaN added this to the 5.6 milestone Dec 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the test suite to reduce code duplication by introducing a factory-like helper method for creating test User instances. The changes replace multiple identical inline model instantiations with calls to a centralized makeUser() method, improving maintainability and consistency.

Key changes:

  • Added makeUser() helper method to eliminate repetitive User model instantiation
  • Added setUp() method to properly initialize Carbon test time
  • Added missing return type declarations to test methods
Comments suppressed due to low confidence (1)

tests/ModelTest.php:1

  • The screenshot filename references '2025-06-24', which is a future date (current date is December 8, 2025). This appears to be an incorrect timestamp in the screenshot filename.
<?php

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

public function testInsert(): void
private function makeUser(): User
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The helper method makeUser() has private visibility, which prevents subclasses from reusing this factory method. Consider changing it to protected to allow test class inheritance and reuse of the helper.

Suggested change
private function makeUser(): User
protected function makeUser(): User

Copilot uses AI. Check for mistakes.
Comment on lines 100 to +107
$user->save();

return $user;
}

public function testInsert(): void
{
$user = $this->makeUser();
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The makeUser() method calls save() which performs a database operation. This tightly couples test data creation with persistence. Consider separating concerns by having the method only construct the user object and letting individual tests decide when to save, or alternatively rename to createUser() to better reflect that it persists data.

Suggested change
$user->save();
return $user;
}
public function testInsert(): void
{
$user = $this->makeUser();
return $user;
}
public function testInsert(): void
{
$user = $this->makeUser();
$user->save();

Copilot uses AI. Check for mistakes.
@GromNaN GromNaN merged commit 6d7c860 into mongodb:5.x Dec 8, 2025
30 checks passed
@GromNaN
Copy link
Member

GromNaN commented Dec 8, 2025

Thank you @zigzagdev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants