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

More info on failure to parse dataProvider #4439

Merged
merged 23 commits into from Aug 24, 2017

Conversation

Projects
None yet
4 participants
@sh41
Contributor

sh41 commented Aug 9, 2017

Provide exception file location, line number and message when thrown during dataProvider execution.

An example failure: Missing extension for an included module.
Current output:

  [Codeception\Exception\TestParseException]
  Couldn't parse test '/var/www/html/tests/Unit/TestCest.php'
  DataProvider 'myProvider' for TestCest->testSomeThings is invalid or not callable.
  Make sure that the dataprovider exist within the test class.

New output:

  [Codeception\Exception\TestParseException]
  Couldn't parse test '/var/www/html/tests/Unit/TestCest.php'
  DataProvider 'myProvider' for TestCest->testSomeThings is invalid or not callable.
  Make sure that the dataprovider exist within the test class.
  /var/www/html/vendor/phpseclib/phpseclib/phpseclib/Math/BigInteger.php:90:GMP is not setup correctly on this system

sh41 added some commits Jun 16, 2017

Enable the "disabled" mode for symfony/phpunit-bridge
Deal with disabled mode for symfony/phpunit-bridge in the same way that symfony/phpunit-bridge bootstrap does as seen in symfony/symfony@1b21647#diff-81bfee6017752d99d3119f4ddb1a09edR27
Merge pull request #1 from sh41/patch-1
Enable the "disabled" mode for symfony/phpunit-bridge
More info on failure to parse dataProvider
Provide exception file location, line number and message when an thrown during dataProvider execution.
@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Aug 9, 2017

Member

In your example additional message provides conflicting information with the main message.

Would it be possible to replace catch(Exception) with more specific catch matching only invalid data providers or implement a better validation?

Member

Naktibalda commented Aug 9, 2017

In your example additional message provides conflicting information with the main message.

Would it be possible to replace catch(Exception) with more specific catch matching only invalid data providers or implement a better validation?

Instead of catching all exceptions, only catch the exceptions that re…
…late to execution of the reflected dataProvider method. The effect of this is to still provide useful information for when the dataProvider isn't specified correctly, but when there is an actual exception thrown in the execution of the dataProvider function it will not be caught and will allow the user to see the problem in the same way as an uncaught exception in an test would on stderr.
@Naktibalda

Thanks for simple solution and good test coverage.

Show outdated Hide outdated tests/cli/RunCest.php

sh41 added some commits Aug 11, 2017

Create new suite based on dummy suite to run these tests in. The dumm…
…y suite needs to be able to run without failure and these Cests deliberatley introduce failures so that we can test them.
Do not include the test suite that deliberately throws exceptions in …
…the tests for re-running failed tests. Re-running a failed test with an exception results in the exception being thrown in the re-run.
Make failing tests clean up after themselves so that there are no sid…
…e effects in other tests. The failing tests seem to end up in the "failed" group and get re-run.

sh41 added some commits Aug 11, 2017

app.wercker.com and appveyor behave differently in their output of th…
…e Application headers when stderr is redirected.
Do not include the test suite that deliberately throws exceptions in …
…the tests for re-running failed tests. Re-running a failed test with an exception results in the exception being thrown in the re-run.
@sh41

This comment has been minimized.

Show comment
Hide comment
@sh41

sh41 Aug 11, 2017

Contributor

Apologies for the high numbers of commits - I seem to get very different results from the CI environments than I do on my dev box. Is there a way of running the CI tests without having to push to github each time?

Contributor

sh41 commented Aug 11, 2017

Apologies for the high numbers of commits - I seem to get very different results from the CI environments than I do on my dev box. Is there a way of running the CI tests without having to push to github each time?

@sh41

This comment has been minimized.

Show comment
Hide comment
@sh41

sh41 Aug 11, 2017

Contributor

I'm going to rethink how these tests are being executed. Their inclusion in the claypit with all the other tests seems to be causing a lot of knock on trouble.

The latest issue is with the TestLoaderTest routine which attempts to load all tests in the claypit. The exception being thrown by one of the new tests is causing issues with loading and as far as I can see there is no way to sensibly exclude a folder from the routine being tested.

Do you have any suggestions @Naktibalda? I am thinking that I will look at refactoring into a different directory so that they do not get included by other tests that pick up the all of the claypit suites?

Contributor

sh41 commented Aug 11, 2017

I'm going to rethink how these tests are being executed. Their inclusion in the claypit with all the other tests seems to be causing a lot of knock on trouble.

The latest issue is with the TestLoaderTest routine which attempts to load all tests in the claypit. The exception being thrown by one of the new tests is causing issues with loading and as far as I can see there is no way to sensibly exclude a folder from the routine being tested.

Do you have any suggestions @Naktibalda? I am thinking that I will look at refactoring into a different directory so that they do not get included by other tests that pick up the all of the claypit suites?

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Aug 11, 2017

Member

Don't put your tests to claypit (I think that it is only necessary for testing generator commands),
put your test or suite to tests/data

Member

Naktibalda commented Aug 11, 2017

Don't put your tests to claypit (I think that it is only necessary for testing generator commands),
put your test or suite to tests/data

sh41 added some commits Aug 13, 2017

Revert "Comments to aid understanding of what is going on and removal…
… of redundant tests from stdout."

This reverts commit ea4ce40.
Revert "Comments to aid understanding of what is going on and removal…
… of redundant tests from stdout."

This reverts commit ea4ce40.
Instead of catching all exceptions during reflection and execution of…
… dataProvider methods, only catch the exceptions that relate to execution of the reflected dataProvider method. The effect of this is to still provide useful information for when the dataProvider isn't specified correctly, but when there is an actual exception thrown in the execution of the dataProvider function it will not be caught and will allow the user to see the problem in the same way as an uncaught exception in an test would on stderr.
@sh41

This comment has been minimized.

Show comment
Hide comment
@sh41

sh41 Aug 14, 2017

Contributor

Have reverted all changes made to claypit/RunCest and refactored the sub tests into a separate suite and tests and added a new Cest separate from RunCest to handle execution of them. Hopefully this will provide appropriate separation so that the execution doesn't effect other suites/tests.

Should I squash the commits in this PR? I've made quite a mess of the logs.

Contributor

sh41 commented Aug 14, 2017

Have reverted all changes made to claypit/RunCest and refactored the sub tests into a separate suite and tests and added a new Cest separate from RunCest to handle execution of them. Hopefully this will provide appropriate separation so that the execution doesn't effect other suites/tests.

Should I squash the commits in this PR? I've made quite a mess of the logs.

PHPUnit version numbers can include alphas, dashes and possibly more.…
… Regexp adjusted to accept any characters as version number.
@sh41

This comment has been minimized.

Show comment
Hide comment
@sh41

sh41 Aug 16, 2017

Contributor

Travis build has failed, with this message:

Failed to clone the git@github.com:twitter/typeahead.js.git repository, try running in interactive mode so that you can enter your GitHub credentials
                                                                                                                                        
  [Composer\Repository\InvalidRepositoryException]                                                                                      
  No valid bower.json was found in any branch or tag of https://github.com/twitter/typeahead.js.git, could not load a package from it.  
                                                                                                                                        

I don't think I've caused this? Is there something I need to do to get Travis to pass?

Contributor

sh41 commented Aug 16, 2017

Travis build has failed, with this message:

Failed to clone the git@github.com:twitter/typeahead.js.git repository, try running in interactive mode so that you can enter your GitHub credentials
                                                                                                                                        
  [Composer\Repository\InvalidRepositoryException]                                                                                      
  No valid bower.json was found in any branch or tag of https://github.com/twitter/typeahead.js.git, could not load a package from it.  
                                                                                                                                        

I don't think I've caused this? Is there something I need to do to get Travis to pass?

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Aug 16, 2017

Member

It is probably yii-basic-app causing issues again.
I restarted the build before checking, but I will look at the output if it fails again.

Member

Naktibalda commented Aug 16, 2017

It is probably yii-basic-app causing issues again.
I restarted the build before checking, but I will look at the output if it fails again.

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Aug 16, 2017

Member

@samdark yii2 is breaking Codeception builds again.

Installing yiisoft/yii2-app-basic (2.0.12)
  - Installing yiisoft/yii2-app-basic (2.0.12): Downloading (100%)
Created project in frameworks-yii-basic
Loading composer repositories with package information
Updating dependencies
Failed to clone the git@github.com:jquery/jquery-dist.git repository, try running in interactive mode so that you can enter your GitHub credentials
                                                                                                                                      
  [Composer\Repository\InvalidRepositoryException]                                                                                    
  No valid bower.json was found in any branch or tag of https://github.com/jquery/jquery-dist.git, could not load a package from it. 
Member

Naktibalda commented Aug 16, 2017

@samdark yii2 is breaking Codeception builds again.

Installing yiisoft/yii2-app-basic (2.0.12)
  - Installing yiisoft/yii2-app-basic (2.0.12): Downloading (100%)
Created project in frameworks-yii-basic
Loading composer repositories with package information
Updating dependencies
Failed to clone the git@github.com:jquery/jquery-dist.git repository, try running in interactive mode so that you can enter your GitHub credentials
                                                                                                                                      
  [Composer\Repository\InvalidRepositoryException]                                                                                    
  No valid bower.json was found in any branch or tag of https://github.com/jquery/jquery-dist.git, could not load a package from it. 
@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Aug 24, 2017

Member

Wow, thanks for the work and so many nice tests! Finally we got CI issues sorted out so I think this PR can be merged

Member

DavertMik commented Aug 24, 2017

Wow, thanks for the work and so many nice tests! Finally we got CI issues sorted out so I think this PR can be merged

@DavertMik DavertMik merged commit 84fd3a2 into Codeception:2.3 Aug 24, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
semaphoreci The build passed on Semaphore.
Details
wercker/build Wercker pipeline passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment