Skip to content

Conversation

@bocharsky-bw
Copy link
Contributor

No description provided.

@javiereguiluz javiereguiluz changed the title Move PHPUnit tests folder into the rood folder Move PHPUnit tests folder into the root folder Jul 9, 2016
@javiereguiluz
Copy link
Member

👍

@javiereguiluz
Copy link
Member

I agree with this change, but I'd like to read other opinions. Apparently, Symfony doesn't recommend this practice officially.

@bocharsky-bw
Copy link
Contributor Author

bocharsky-bw commented Jul 10, 2016

I'm wondering to hear other opinions too. BTW, symfony-standard already moved DefaultControllerTest into the tests directory:
https://github.com/symfony/symfony-standard/blob/master/tests/AppBundle/Controller/DefaultControllerTest.php
And autoloads tests from tests dir in composer.json

@voronkovich
Copy link
Contributor

I think that holding the folder with tests in the root directory just makes it easier to launch the tests: phpunit vs. phpunit -c app. 👍

@javiereguiluz
Copy link
Member

@voronkovich good point ... but if I'm right, that's achieved by moving the phpunit.xml.dist config file to the root directory. That's what Symfony 3.x did and we can run the tests with phpunit even if they are located inside src/

@voronkovich
Copy link
Contributor

@javiereguiluz, you're right, I mistakenly confused the test directory and phpunit.xml file. So, I don't have any opinion about this PR :(

@voronkovich
Copy link
Contributor

@javiereguiluz, there is an explanation why tests folder was moved to the root directory in the symfony-standard. See symfony/symfony-standard@d0311d2#commitcomment-15653984

@xabbuh
Copy link
Member

xabbuh commented Jul 11, 2016

The namespace of the SluggerTest class needs to be updated too.

@bocharsky-bw
Copy link
Contributor Author

bocharsky-bw commented Jul 11, 2016

@xabbuh Yes, I know. It's weird, but it already has a right namespace in master. Actually, I think we have a bug now in master, but this PR should fix it.

@xabbuh
Copy link
Member

xabbuh commented Jul 11, 2016

@bocharsky-bw But the namespace should be Tests\AppBundle\Utils, shouldn't it?

@xabbuh
Copy link
Member

xabbuh commented Jul 11, 2016

And to be consistent with the Symfony Standard Edition we would need to have a tests/AppBundle directory.

@bocharsky-bw
Copy link
Contributor Author

@xabbuh Ah, you're totally right, I missed AppBundle.

Thanks for notice it! I'll fix it soon.

@bocharsky-bw
Copy link
Contributor Author

Done.

@xabbuh
Copy link
Member

xabbuh commented Jul 12, 2016

👍

@javiereguiluz
Copy link
Member

Thank you @bocharsky-bw.

@bocharsky-bw bocharsky-bw deleted the tests branch July 26, 2016 19:46
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.

4 participants