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

Automatic registration of server env vars #272

Closed

Conversation

ZeeCoder
Copy link
Contributor

I implemented the solution we aggreed upon in issue #270.
The problem is, that it breaks an existing test, and I can't figure out why.

Can someone help me out with this?
(If this issue is resolved I'll add tests for the new functionality too.)

Here's the output of phpunit:

PHPUnit 4.6.6 by Sebastian Bergmann and contributors.

Configuration read from /home/zeecoder/work/deployer/phpunit.xml

............................................F.................... 65 / 74 ( 87%)
.........

Time: 2.13 seconds, Memory: 12.75Mb

There was 1 failure:

1) Deployer\Server\BuilderTest::testEnv
Expectation failed for method name is equal to <string:set> when invoked 1 time(s)
Parameter 0 for invocation Deployer\Server\Environment::set('server', Array (...)) does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'name'
+'server'

/home/zeecoder/work/deployer/src/Server/Builder.php:36
/home/zeecoder/work/deployer/test/src/Server/BuilderTest.php:120
/usr/share/php/PHPUnit/TextUI/Command.php:186
/usr/share/php/PHPUnit/TextUI/Command.php:138

FAILURES!
Tests: 74, Assertions: 208, Failures: 1.

@antonmedv
Copy link
Member

It's fail down because of (BuilderTest.php#L115)[https://github.com/deployphp/deployer/blob/master/test/src/Server/BuilderTest.php#L115]
In this test Deployer\Server\Environment expected to be called once. And you add second call in this PR.

@ZeeCoder
Copy link
Contributor Author

I think it should be okay now.

@antonmedv
Copy link
Member

Please, before i merge this PR, add docs to deployphp/docs too. Thank a lot!

@antonmedv
Copy link
Member

Now we need set as private here.

@ZeeCoder ZeeCoder closed this May 24, 2015
@ZeeCoder ZeeCoder deleted the feature/auto-server-env-vars branch May 24, 2015 19:34
@ZeeCoder
Copy link
Contributor Author

I had to redo this due to incompatibilities with the resent changes.
Also it contained 5 commits for something really simple.

antonmedv pushed a commit that referenced this pull request May 25, 2015
Refactored PR based on #272 (automatic setting of the "server" env var)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants