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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Purifies model factories #12700

Merged
merged 41 commits into from
Mar 21, 2023
Merged

Conversation

marcusmoore
Copy link
Collaborator

Description

This PR "purifies" model factories as much as possible to pave the way for their use in test cases easily and with minimal side-effects (mainly avoiding the need to call them in specific and bespoke ways).

Previously, most model factories were dependent on data to be seeded in a specific way and relationships within the factories used hardcoded ids. To use a factory you would need to either seed the necessary data or set the relationship manually in each test even when you didn't particularly care about the related model.

Now, each model factory can be called in isolation using {Model}::factory()->create() 馃帀

In the process of making these changes, seeders have been updated and licenses are checked out to users in the same way that assets are when the seeders are run.

Details

Defining Related Models

Many calls like the following have been replaced:

- 'location_id' => rand(1, 5),
+ 'location_id' => Location::factory(),

Instead of having to seed five locations or manually set the location_id in tests, the Accessory factory can be called without having to think about the associated Location.

In some instances, a model or model state requires a specific related model to exist and in those cases closures are used to handle the relationship:

// The appleBtKeyboard state of an Accessory:
'category_id' => function () {
  return Category::where('name', 'Keyboards')->first() ?? Category::factory()->accessoryKeyboardCategory();
},

Defining default required fields

Missing required fields and relationships, qty in AccessoryFactory for example, have been added.

Seeder Changes

Some seeders still require other data to exist before they can be run. Naive checks at the top of the relevant seeders have been added to account for this:

if (! Asset::count()) {
  $this->call(AssetSeeder::class);
}

Tiny Test

A tiny test has been added to ensure the Asset factory can be called as expected (tests/Feature/Api/Assets/AssetIndexTest.php).

General Clean Ups

  • Factory states that called create() instead of setting state have been updated (f.e. Category::factory()->assetLaptopCategory())
  • Missing ()s have been added to faker calls since this syntax ($this->faker->sentence) is depreciated and should be called as $this->faker->sentence()
  • The accessoryKeyboardCategory state in Category's name has been fixed (Keyboardss => Keyboards)
  • Simplified fully qualified namespace uses (\App\Models\Asset::class => Asset::class)
  • Removed unneeded imports

Before and Afters

Item Before After
Seeding Total Time: 0m36.926s

SettingsSeeder (16.26ms)
CompanySeeder (26.73ms)
CategorySeeder (34.32ms)
LocationSeeder (21.59ms)
UserSeeder (276.35ms)
DepreciationSeeder (10.66ms)
DepartmentSeeder (10.51ms)
ManufacturerSeeder (44.29ms)
SupplierSeeder (16.63ms)
AssetModelSeeder (40.10ms)
DepreciationSeeder (9.73ms)
StatuslabelSeeder (14.99ms)
AccessorySeeder (21.52ms)
AssetSeeder (19,159.39ms)
LicenseSeeder (70.12ms)
ComponentSeeder (39.39ms)
ConsumableSeeder (13.81ms)
ActionlogSeeder (13,612.08ms)
CustomFieldSeeder (848.16ms)
Total Time: 0m14.641s

SettingsSeeder (23.28ms)
CompanySeeder (19.92ms)
CategorySeeder (30.39ms)
LocationSeeder (23.27ms)
DepartmentSeeder (11.47ms)
UserSeeder (174.96ms)
DepreciationSeeder (5.98ms)
ManufacturerSeeder (62.30ms)
SupplierSeeder (20.74ms)
AssetModelSeeder (53.74ms)
DepreciationSeeder (4.12ms)
StatuslabelSeeder (6.69ms)
AccessorySeeder (16.20ms)
AssetSeeder (7,102.35ms)
LicenseSeeder (16.15ms)
ComponentSeeder (10.02ms)
ConsumableSeeder (9.90ms)
ActionlogSeeder (3,538.21ms)
CustomFieldSeeder (1,057.00ms)
Models created Accessory - 4
Actionlog - 400
Asset - 1373
AssetMaintenance - 0
AssetModel - 18
Category - 15
CheckoutAcceptance - 0
CheckoutRequest - 0
Company - 7
Component - 4
Consumable - 3
ConsumableAssignment - 0
CustomField - 5
CustomFieldset - 2
Department - 6
Depreciation - 3
Group - 0
Import - 0
License - 4
LicenseSeat - 50
Location - 1762
Manufacturer - 11
PredefinedKit - 0
SCIMUser - 58
Setting - 1
Statuslabel - 7
Supplier - 1718
User - 58
Accessory - 4
Actionlog - 420
Asset - 1373
AssetMaintenance - 0
AssetModel - 18
Category - 15
CheckoutAcceptance - 0
CheckoutRequest - 0
Company - 8
Component - 4
Consumable - 3
ConsumableAssignment - 0
CustomField - 5
CustomFieldset - 2
Department - 6
Depreciation - 3
Group - 0
Import - 0
License - 4
LicenseSeat - 50
Location - 10
Manufacturer - 11
PredefinedKit - 0
SCIMUser - 58
Setting - 1
Statuslabel - 7
Supplier - 5
User - 58
Tests 54 tests - Time: 8.54s 55 tests - 8.63s

@what-the-diff
Copy link

what-the-diff bot commented Mar 21, 2023

PR Summary

  • Updated Factory Methods - Improved the use of factory methods across multiple factories, making testing and development more efficient.
  • Removed Unused Code and Fixed Typos - Cleaned up the codebase by removing unused code and fixing typos in comments and documentation.
  • Improved Importing and Namespacing - Better importing of models and usage of fully qualified namespaces instead of aliases for more clarity and efficiency.
  • Replaced Hardcoded Data - Replaced hardcoded data with dynamic factory generated values, preventing duplicate records and supporting better testing.
  • Updated Seeder Files - Adjusted the seeder files to make use of factories and ensure prerequisites were properly seeded before assets.
  • New and Updated Tests - Added tests for asset index endpoint, adjusted existing tests to use factories and account for changes in the codebase.

@snipe
Copy link
Owner

snipe commented Mar 21, 2023

This looks fantastic - thank you!

@snipe snipe merged commit 342b399 into snipe:develop Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants