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

Add few more dashes #2

Merged
merged 1 commit into from
May 12, 2022
Merged

Add few more dashes #2

merged 1 commit into from
May 12, 2022

Conversation

TomasVotruba
Copy link
Contributor

No description provided.

@TomasVotruba TomasVotruba changed the title add few more dashes Add few more dashes May 12, 2022
@alexander-schranz alexander-schranz merged commit 20d7c82 into main May 12, 2022
@alexander-schranz alexander-schranz deleted the tv-first-rule-fixup branch May 12, 2022 15:38
@alexander-schranz
Copy link
Member

@TomasVotruba Thank you ❤️

@TomasVotruba
Copy link
Contributor Author

You're welcome 👍

I can say you've done the setup propperly, the config and the tests are as they should be, so is the autoloading.
One little improvement can be using correct PSR-4 namespace in a test class (now there is Symfony, probably from old copy-paste), and in test fixture files. PHPStan can in some cases mix two class SomeClass {} from different locations. So it(s better to separate them by fixture namespace.

But that's just little detail that might help you later on. Otherwise very good job 👏

@alexander-schranz
Copy link
Member

Thank you for your feedback 😃.

I really like the test setup you created here, it make it really easy to write tests. I cloned the setup for my other project a test-generator which tests work the same way. Example here. And inspired by your setup using markdown for API tests which work the same way here.

Add a namespace make sense. Did fix that and the Namespace in #3.

@TomasVotruba
Copy link
Contributor Author

TomasVotruba commented May 12, 2022

Cool 👍

I started to use these tests after I learned about them in nikic/php-parser.

Thanks to uniform design, you can do cool tricks like:

Add a namespace make sense. Did fix that and the Namespace in #3.

👍

@alexander-schranz
Copy link
Member

@TomasVotruba Nice, thank you for sharing. I also thought about the update thing, because I know it from Jest which has also this kind of snapshots tests: https://jestjs.io/docs/snapshot-testing. And they also have this kind of flag to update the tests via a additional flag.

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