Skip to content

Conversation

@RGO230
Copy link
Contributor

@RGO230 RGO230 commented Oct 20, 2025

@DenTray DenTray assigned RGO230 and unassigned DenTray Oct 23, 2025
@DenTray DenTray requested a review from Copilot October 23, 2025 07:48
Copy link
Contributor

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 generator code to use resource names (e.g., NovaWelcomeBonusResource) instead of model names (e.g., WelcomeBonus) when naming fixture directories and files. The change ensures consistency between test class names and their associated fixture paths.

Key Changes

  • Introduced an $entity property to track the resource/model name used for fixture generation
  • Updated fixture paths to use resource names (e.g., NovaWelcomeBonusResourceTest directory instead of NovaWelcomeBonusTest)
  • Modified fixture file naming to include the full resource name (e.g., nova_welcome_bonus_resource_dump.sql instead of nova_welcome_bonus_dump.sql)

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/NovaTestGeneratorTest.php Updated test assertions to expect fixture paths and filenames with resource names
tests/CommandTest.php Updated test assertions to reflect new fixture directory and filename patterns
src/Generators/TestsGenerator.php Added generate() method to set $entity property from model name
src/Generators/NovaTestGenerator.php Modified to use $entity property for resource name throughout fixture generation
src/Generators/AbstractTestsGenerator.php Removed local $entity variable and updated to use class property for fixture naming

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@RGO230 RGO230 assigned DenTray and unassigned RGO230 Oct 23, 2025
@RGO230 RGO230 assigned AZabolotnikov and unassigned DenTray Oct 27, 2025
$excepts = ($modification === 'request') ? ['id'] : [];

$this->generateFixture("{$type}_{$entity}_{$modification}.json", Arr::except($object, $excepts));
$this->generateFixture("{$type}_" . Str::snake($this->getTestingEntityName()) . "_{$modification}.json", Arr::except($object, $excepts));
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert

@RGO230 RGO230 assigned AZabolotnikov and unassigned RGO230 Nov 2, 2025
Comment on lines 47 to 53
$entityName = $this->getTestingEntityName();

if ($this->classExists('nova', "Nova{$entityName}Test")) {
$path = $this->getClassPath('nova', "Nova{$entityName}Test");

throw new ResourceAlreadyExistsException($path);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reasons to make it after searching the nova resource in the filesystem???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the logic of the abstract method getTestingEntityName depends on the filesystem scan


$entityName = $this->getTestingEntityName();

if ($this->classExists('nova', "Nova{$entityName}Test")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if ($this->classExists('nova', "Nova{$entityName}Test")) {
if ($this->classExists('nova', $this->getTestClassName())) {

$entityName = $this->getTestingEntityName();

if ($this->classExists('nova', "Nova{$entityName}Test")) {
$path = $this->getClassPath('nova', "Nova{$entityName}Test");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$path = $this->getClassPath('nova', "Nova{$entityName}Test");
$path = $this->getClassPath('nova', $this->getTestClassName());

@DenTray DenTray assigned RGO230 and unassigned DenTray Nov 4, 2025
@DenTray DenTray requested a review from Copilot November 4, 2025 05:49
Copy link
Contributor

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


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

->setModel('WelcomeBonus')
->generate();

$this->assertFileDoesNotExist('tests/NovaWelcomeBonusTest.php');
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The assertion checks for the old test class name NovaWelcomeBonusTest which is inconsistent with the new naming convention. Since the Nova resource is WelcomeBonusResource, the expected test class name should be NovaWelcomeBonusResourceTest. Consider updating this assertion to verify the correct behavior by checking that tests/NovaWelcomeBonusResourceTest.php does not exist when the stub is missing.

Suggested change
$this->assertFileDoesNotExist('tests/NovaWelcomeBonusTest.php');
$this->assertFileDoesNotExist('tests/NovaWelcomeBonusResourceTest.php');

Copilot uses AI. Check for mistakes.
$this->novaResourceClassName = Arr::first($novaResources);

$entityName = $this->getTestingEntityName();

Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

There's trailing whitespace on line 48. Remove the extra whitespace at the end of this line for consistency with project formatting standards.

Suggested change

Copilot uses AI. Check for mistakes.
@RGO230 RGO230 assigned AZabolotnikov and unassigned RGO230 Nov 4, 2025
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.

4 participants