Fix small refactoring issue #4000

Merged
merged 2 commits into from Feb 7, 2017

Conversation

Projects
None yet
3 participants
@lcobucci
Contributor

lcobucci commented Feb 6, 2017

On ed549a9 an else statement was removed (awesome) but the author forgot to interrupt the execution flow and that is causing our test suite to fail 馃槶 but this fixes it.

Fixes #4001

Fix small refactoring issue
On ed549a9 an `else` statement was
removed (awesome) but the author forgot to interrupt the execution
flow and that is causing our test suite to fail 馃槶 but this fixes
it.
@lcobucci

This comment has been minimized.

Show comment
Hide comment
@lcobucci

lcobucci Feb 6, 2017

Contributor

The sad thing is that this issue wasn't detected by your test suite and since I'm not familiar with the codebase I don't know how can I help you to make sure this won't happen again...

Contributor

lcobucci commented Feb 6, 2017

The sad thing is that this issue wasn't detected by your test suite and since I'm not familiar with the codebase I don't know how can I help you to make sure this won't happen again...

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Feb 7, 2017

Member

The sad thing is that this issue wasn't detected by your test suite and since I'm not familiar with the codebase I don't know how can I help you to make sure this won't happen again...

Yes, that's really sad (
iframe was not tested properly. However, it is not that hard to make a regression test:

You need to start demo application
Create a web page with iframes in tests/data/app/form
Write a test at tests/web/WebDriverTest.php

Member

DavertMik commented Feb 7, 2017

The sad thing is that this issue wasn't detected by your test suite and since I'm not familiar with the codebase I don't know how can I help you to make sure this won't happen again...

Yes, that's really sad (
iframe was not tested properly. However, it is not that hard to make a regression test:

You need to start demo application
Create a web page with iframes in tests/data/app/form
Write a test at tests/web/WebDriverTest.php

@lcobucci

This comment has been minimized.

Show comment
Hide comment
@lcobucci

lcobucci Feb 7, 2017

Contributor

@DavertMik tests added by @snoek09 (he was much faster than me 馃槃)

I think is now ready to go 馃槈

Contributor

lcobucci commented Feb 7, 2017

@DavertMik tests added by @snoek09 (he was much faster than me 馃槃)

I think is now ready to go 馃槈

@Naktibalda Naktibalda merged commit 41a206e into Codeception:2.2 Feb 7, 2017

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
wercker/build Wercker pipeline passed
Details
@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Feb 7, 2017

Member

Thank you @lcobucci and @snoek09
Awesome work!

Member

DavertMik commented Feb 7, 2017

Thank you @lcobucci and @snoek09
Awesome work!

@lcobucci lcobucci deleted the lcobucci:fix-webdriver-issue branch Feb 8, 2017

@lcobucci

This comment has been minimized.

Show comment
Hide comment
@lcobucci

lcobucci Feb 9, 2017

Contributor

@DavertMik @Naktibalda any plan on releasing it?

Contributor

lcobucci commented Feb 9, 2017

@DavertMik @Naktibalda any plan on releasing it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment