magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 154 forks source link

Resolves bug reported in Issue #76 #83

Closed davidalger closed 10 years ago

davidalger commented 10 years ago

Resolves the bug reported in issue #76 and as far as I've been able to verify, doesn't break the copy method in other use cases. If you find I've broken it, let me know and I'll go back to the drawing board… those trailing slashes were being rather pesky. ;)

Flyingmana commented 10 years ago

as you can see, you broke the unittests somehow, could you look yourself into the source of the problem?

davidalger commented 10 years ago

I'm working on resolving those issues. Somehow I didn't notice it had unit tests to run… doh! :/ Anyhow, I'm running them locally to check things now, and may even add a unit test for the big which this is supposed to resolve. ;)

@CNanninga - Tagging you here so you know and will get this thread…

Flyingmana commented 10 years ago

maybe helpfully, I added some testing with https://github.com/magento-hackathon/magento-composer-installer/blob/master/tests/MagentoHackathon/Composer/Magento/FullStackTest.php to better test problems with real existing modules and created or not created files as result of normal usage.

davidalger commented 10 years ago

Thanks. I'll check that out and definitely give it a go. There are public OSS modules which are affected by the bug here, but I've got a unit test which I'm testing against for implementation now. :)

davidalger commented 10 years ago

@Flyingmana Question: How can I get the artifact needed to successfully run the full stack tests? If I merge master into this branch and then run phpunit I get the following output:

There was 1 failure:

1) MagentoHackathon\Composer\Magento\FullStackTest::testFirstInstall
Failed asserting that file "/Volumes/Server/proj/magento-composer-installer/tests/FullStackTest/artifact/magento-hackathon-magento-composer-installer-999.0.0.zip" exists.

/Volumes/Server/proj/magento-composer-installer/tests/MagentoHackathon/Composer/Magento/FullStackTest.php:109
phar:///usr/local/Cellar/phpunit/3.7.28/libexec/phpunit-3.7.28.phar/phpunit/TextUI/Command.php:176
phar:///usr/local/Cellar/phpunit/3.7.28/libexec/phpunit-3.7.28.phar/phpunit/TextUI/Command.php:129

FAILURES!                                             
Tests: 101, Assertions: 184, Failures: 1, Skipped: 11.

Currently branch issue-76 contains all of tag 2.0.0-beta1 and the unit tests run perfectly on my dev setup. Still waiting to hear back from Travis. :)

Flyingmana commented 10 years ago

that could be probably caused from a missing composer.phar in the path, I use composers archive command to generate the artifact

Flyingmana commented 10 years ago

could you do a merge, to include the latest unittests?

davidalger commented 10 years ago

The latest code in master has been merged in.

Just verified that composer.phar is in my path and executes. What is strange is there are a few other artifacts left behind after the error about magento-hackathon-magento-composer-installer-999.0.0.zip not existing. Any other ideas? I just updated composer to make sure it wasn't due to being on a slightly out of date version, but that didn't affect anything.

davidalger commented 10 years ago

@Flyingmana Do you want me to fix the broken full stack test for this or are you planning on doing that?

Flyingmana commented 10 years ago

I need a bit of time to think about this and look again trough it, but if you fix it, I would merge it and wait if someone complains about the behavior.