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

[10.x] Passthru test options #48335

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Conversation

jasonmccreary
Copy link
Contributor

@jasonmccreary jasonmccreary commented Sep 7, 2023

This is a replacement for #48319 to simply passthru the --test or --pest option to the underlying make:controller call. This allows the controller test to also be created.

In addition, the --test option is automatically set to true when the -a option is used to generate a default test, unless the --pest option is set to specifically generate Pest tests. After sleeping on it, the hard truth is most devs don't write tests. As such, I think forcing tests with the -a option would make "extra" files. Those that write tests can run: make:model -a --test. Once this PR is merged, running that would properly generate a test for the model and the controller.

@nexxai
Copy link
Contributor

nexxai commented Sep 8, 2023

Man, thank you so much for making this so much simpler than I was able to. I've been fighting it all night and yours looks way better.

@nexxai
Copy link
Contributor

nexxai commented Sep 8, 2023

Actually, I spoke too soon. For whatever reason, even if you set $this->input->setOption('test'); or $this->input->setOption('pest'); in the if ($this->option('all')) {... check, the $this->handleTestCreation() method that gets added by the trait never gets called and the test never actually gets created. I've been working on a PR for a bit and I had to create a separate method that called it because I couldn't figure out any other way to trigger it.

@nexxai
Copy link
Contributor

nexxai commented Sep 8, 2023

I won't actually send in my PR, but you can see what I was trying here: 1cbb6da

@jasonmccreary
Copy link
Contributor Author

I definitely like the sniff out. But I'd like wait and see what the core team thinks about adding tests with the -a option in general before I refine.

For me the win is passing the test options down to the controller. Personally, I'm comfortable writing make:model -a --test to be explicit.

@nexxai
Copy link
Contributor

nexxai commented Sep 8, 2023

Totally understand. Thanks for clarifying.

@driesvints driesvints changed the title Passthru test options [10.x] Passthru test options Sep 8, 2023
@taylorotwell taylorotwell merged commit 9ae962a into laravel:10.x Sep 11, 2023
@jasonmccreary jasonmccreary deleted the passthru-tests branch September 12, 2023 00:05
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.

3 participants