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

Fix config tests when environment variables are set #3448

Merged
merged 4 commits into from
Nov 21, 2014

Conversation

cs278
Copy link
Contributor

@cs278 cs278 commented Nov 21, 2014

Composer reads environment variables to overload config variables but the tests do not expect this, so the following incantation fails:

$ COMPOSER_CACHE_DIR=/tmp vendor/bin/phpunit --stop-on-failure

PHPUnit 3.7.37 by Sebastian Bergmann.

Configuration read from /home/chris.smith/src/composer/phpunit.xml.dist

.............................................................   61 / 1245 (  4%)
..................................F

Time: 1.99 seconds, Memory: 25.75Mb

There was 1 failure:

1) Composer\Test\ConfigTest::testVarReplacement
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/home/chris.smith/foo'
+'/tmp'

/home/chris.smith/src/composer/tests/Composer/Test/ConfigTest.php:121

FAILURES!
Tests: 96, Assertions: 362, Failures: 1.

This PR actually has two resolutions implemented, see 725a4fd. I think the second implementation makes more sense, I will remove the first two commits out if that's the consensus.

@stof
Copy link
Contributor

stof commented Nov 21, 2014

IMO, resetting the environment in the test to ensure it is clean is a better solution than adding an option in the code to be able to ignore the environment

@cs278
Copy link
Contributor Author

cs278 commented Nov 21, 2014

IMO, resetting the environment in the test to ensure it is clean is a better solution than adding an option in the code to be able to ignore the environment

I was following that thinking initially, but then considered that disabling the access of global state would was a better option and could potentially be reused elsewhere.

@naderman
Copy link
Member

Indeed, who knows what else this may end up being useful for and it's not like it's a huge amount of code to enable this.

naderman added a commit that referenced this pull request Nov 21, 2014
Fix config tests when environment variables are set
@naderman naderman merged commit b21f2be into composer:master Nov 21, 2014
@cs278 cs278 deleted the config-env-test branch November 22, 2014 00:00
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