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

Initial implementation of Behat ListFeaturesExtension and FastestEnviron... #7

Closed
wants to merge 1 commit into from

Conversation

diegosainz
Copy link

This PR is not meant to be integrated (tests are still missing) - I just want to make sure to have your opinion before working further in it.

I added many sections to the README, explaining some scenarios on how to use the implementation under Mink.

@liuggio
Copy link
Owner

liuggio commented Nov 8, 2014

seems great to me, now I'm travelling in few hours I'll be at home, and I'd go deeper into the code

@liuggio
Copy link
Owner

liuggio commented Nov 8, 2014

great to me 👍

@diegosainz
Copy link
Author

Added tests for Behat extension and updated travis file.

The PHPUnit test for Environment/FastestEnvironment.php is not yet there.

@liuggio
Copy link
Owner

liuggio commented Nov 17, 2014

yesss pretty well.

is not merge-able :(


To install the extension just add it to your `behat.yml` file:

# behat.yml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not parsed correctly

missing space or ```yml

@diegosainz
Copy link
Author

Ups - sorry about that. It should be fixed now.

@liuggio
Copy link
Owner

liuggio commented Nov 17, 2014

Great!
is not mergeable,
could you also squash the 3 commits into one with git rebase?
thanks a lot for the missing feature

@diegosainz
Copy link
Author

My pleasure to be able to help :) - Commits squashed. I also added FastestEnvironmentTest and modified the bootstrap file to allow the @runTestsInSeparateProcesses annotation (we need that to isolate the tests that change the environment and $_GET/$_SERVER globals).

@diegosainz
Copy link
Author

Just noticed the broken tests because the change I made to bootstrap. After fixing it I got a sh permission error. Maybe it has to do something with the @runTestsInSeparateProcesses phpunit tag and how fastest forks processes?

- 9 shuffled tests into the queue.
- Will be consumed by 8 parallel Processes.
1/9 ✔ tests/Fastest/Queue/Infrastructure/InMemoryQueueTest.php
2/9 ✔ tests/Fastest/Process/ProcessFactoryTest.php
3/9 ✔ tests/Fastest/Queue/ReadFromInputAndPushIntoTheQueueTest.php
4/9 ✔ tests/Fastest/Queue/CreateTestsQueueFromPhpUnitXMLTest.php
5/9 ✘ tests/Fastest/Environment/FastestEnvironmentTest.php
6/9 ✔ tests/Fastest/Queue/Infrastructure/InMemoryQueueFactoryTest.php
7/9 ✔ tests/Fastest/Process/ProcessesManagerTest.php
8/9 ✔ tests/Fastest/Process/ProcessesTest.php
9/9 ✔ tests/Fastest/Process/ProcessorCounterTest.php

[4] tests/Fastest/Environment/FastestEnvironmentTest.phpPHPUnit 4.3.5 by Sebastian Bergmann.

Configuration read from /home/diego/code/git/fastest/phpunit.xml.dist

EEEEE

Time: 107 ms, Memory: 5.00Mb

There were 5 errors:

1) Liuggio\Fastest\Environment\FastestEnvironmentTest::shouldSetEnvironmentVariablesFromGet
PHPUnit_Framework_Exception: sh: 1: : Permission denied

2) Liuggio\Fastest\Environment\FastestEnvironmentTest::shouldSetEnvironmentVariablesFromCookie
PHPUnit_Framework_Exception: sh: 1: : Permission denied

3) Liuggio\Fastest\Environment\FastestEnvironmentTest::shouldSetEnvironmentVariablesFromHttpHeader
PHPUnit_Framework_Exception: sh: 1: : Permission denied

4) Liuggio\Fastest\Environment\FastestEnvironmentTest::shouldNotSetEnvironmentVariablesIfWhenNotPresentInRequest
PHPUnit_Framework_Exception: sh: 1: : Permission denied

5) Liuggio\Fastest\Environment\FastestEnvironmentTest::shouldRaiseExceptionWhenRequestValueIsInvalid
PHPUnit_Framework_Exception: sh: 1: : Permission denied

@diegosainz
Copy link
Author

OK - I removed the @runTestsInSeparateProcesses annotation and replaced it with a setup() method to reset the environment variable manually. Still maybe the error with @runTestsInSeparateProcesses should be considered - do you think its a bug?

@liuggio
Copy link
Owner

liuggio commented Nov 18, 2014

woow great!!!!!

the @runTestsInSeparateProcesses is not a fastest annotation and is ignored by fastest, I created a ticket I'll go deep later on this.

I tried to use it with fastest itself i run bin/behat --list-scenarios features | php fastest "./bin/behat {}"
and I got:

[Exception]                         
  Empty input try piping some files. 

and if I do bin/behat --list-scenarios features > file; cat file | php fastest "./bin/behat {}" it works
but the examples fails :(

Do you know why?

@diegosainz
Copy link
Author

Yeah, you are right about the pipe bug... (under my bash scripts that run the tests it runs correctly, but not directly in the command line as you say) it seems that the function stream_set_blocking on /src/Queue/CreateTestsQueueFromSTDIN.php is causing the trouble (it returns too fast because it does not block, even before start receiving data). If you comment it out it will work as expected - what's the reason it is used? (I found many issues with this function under windows and another circumstances)

EDIT: Also with a sleep(1) before the function it works. I think it has to do something with PHP's output buffering that comes from behat (it opens the stdout stream but delays a little while populating it, causing fastest to think that the stream is empty)

@liuggio
Copy link
Owner

liuggio commented Nov 18, 2014

ok, we could remove it. great job, I tweeted about you :)

@liuggio
Copy link
Owner

liuggio commented Nov 18, 2014

I think the pipe should be re-factored,
for now what do you think if we remove the stream_block?

@liuggio liuggio mentioned this pull request Nov 18, 2014
@diegosainz
Copy link
Author

Thank you for the tweet, that was nice :) About removing the stream_block func, yes, I think we can do that. I'll try to make some time today to test out if there are any negative side-effects.

@diegosainz
Copy link
Author

Giulio, I squashed the PR again with a change to the http header names (it was misnamed combining - and _) - I fixed it in the README, the test and the FastestEnvironment class

@liuggio
Copy link
Owner

liuggio commented Nov 19, 2014

👍

@liuggio
Copy link
Owner

liuggio commented Nov 19, 2014

This PR is not meant to be integrated (tests are still missing

is sill WIP?

What do you think to add the fastest on travis for behat ;) ?

@diegosainz
Copy link
Author

I made another PR with the final version, ready to be merged (#12) - travis tests will fail on that branch until merged with master (and the pipe fix)

@diegosainz diegosainz closed this Nov 20, 2014
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.

None yet

3 participants