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

[make:*] generate test classes properly & fix class name details suffix #1561

Closed
wants to merge 1 commit into from

Conversation

jrushlow
Copy link
Collaborator

@jrushlow jrushlow commented May 15, 2024

  • When passing a suffix to Generator::createClassNameDetails(), the suffix is not added to an absolute class name. Our docs say that passing a suffix "...guarantee is on the end of the class". This PR fixes that.

  • Generates tests in the proper App\Tests\ namespace instead of App\Test\


Internals

When generating tests (--with-tests), I was lazy and used generateFile() to create the test class (in part because of the suffix bug) instead of using the correct generateClass() method. This PR fixes that. It's also needed to accomplish #1539 with test classes.

@jrushlow jrushlow added Bug Bug Fix Status: Needs Work Additional work is needed labels May 15, 2024
Comment on lines +156 to +158
if (!str_ends_with($className, $suffix)) {
$className = sprintf('%s%s', $className, $suffix);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm... I don't think we should "fix" this here. One could argue that because you're passing an absolute class name into this method, it should not be altered. Under that assumption - this "fix" would actually be a BC break as it's changing the intended functionality from 9e90cbc#diff-4ee5eaad222ef0782fccd29473a5f1860988b7cf91cdcd0a12628b2377972b36

Chewing on this for a better solution

@jrushlow jrushlow closed this May 20, 2024
@jrushlow jrushlow deleted the bug/class-name-suffix branch May 20, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Needs Work Additional work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant