fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

PHPUnit tests fail under 1.5/develop #1245

Closed user00265 closed 11 years ago

user00265 commented 11 years ago

While running php oil test I came across three particular classes failing tests, which I will explain to the best of my ability below.

See php oil test output here: http://pastie.org/5537888

user00265 commented 11 years ago

For the tests on the Fieldset and Html classes, I can submit a pull request to save some time and effort if the problems are not with the classes but with the tests. The Agent class I can only fix the missing variable that get_browser() uses to detect the UA, otherwise, I am not exactly sure what the issues are with the rest of the failing tests.

kenjis commented 11 years ago

In my environment, all tests pass. Did you change config file?

OK (321 tests, 318 assertions)

User Agent is set for tests in: https://github.com/fuel/core/blob/1.5/develop/tests/agent.php#L31 The code does not work for you?

user00265 commented 11 years ago

This is all I had to do after I noticed (even without the git submodule call below, just checking out 1.5/develop):

git clone --recursive git://github.com/fuel/fuel.git
cd fuel
git checkout 1.5/develop
git submodule foreach git checkout 1.5/develop
php oil test

PHP version: 5.4.9 PHPUnit version: 3.7.10

WanWizard commented 11 years ago

I have a 100% score here too.

As for your issues:

  1. Both Form and Fieldset call Uri::create() to create the links. The only way a leading slash can be added is when the 'base_url' in the config is set to '/'.
  2. The test for Agent set the required $_SERVER variables hardcoded in the test setup. So the correct UA should be present. A correct browscap file most be loaded for agent to function, otherwise the UA can't be matched and the class returns unset data. Perhaps that is the issue?
user00265 commented 11 years ago

Even when I am not modifying the code in any way, shape or form? Clean clone as outlined above. Doesn't make sense.

kenjis commented 11 years ago

I've just checked as the instruction of @user00265 and passed all tests.

PHP version: 5.3.19 PHPUnit version: 3.7.1

PHP version: 5.4.7 PHPUnit version: 3.7.10

kenjis commented 11 years ago

@user00265, what's your platform? I use Ubuntu.

user00265 commented 11 years ago

The distro is Arch Linux, running from the core repositories (aka -- stable).

WanWizard commented 11 years ago

I'm as clueless as you are. I've been looking at Uri::create(), but I can't figure out a situation where a leading slash will be added. There's even an ltrim() in there to make sure it's stripped.

user00265 commented 11 years ago

Easy enough. Chalk it to unknown and close the issue. We all can't figure what's up, so probably it is my setup, one way or another. If I find out a way to reproduce it, I shall re-open the issue.

user00265 commented 11 years ago

Well, I've tried several times, but I can still make it happen. Whether I clone it from http://github.com/fuel/fuel or from the repo below, I can make it happen all the same. I don't know what the issue is, though. As you say, the tests do have a setup function that should be setting those, but the behavior I am seeing is completely different. Does it have anything to do with the FUEL_ENV setting?

Also, please ignore the slash setting on the URLs, though, interestingly enough, the setup should set the config to expected variables, albeit temporarily instead of using whatever is configured at the moment which might kill the test.

Now, even Travis is able to: https://travis-ci.org/user00265/pagoda-fuelphp/jobs/3723994 Cloned from http://github.com/user00265/pagoda-fuelphp

kenjis commented 11 years ago

I've got your failuers from your repos git://github.com/user00265/pagoda-fuelphp.git

FAILURES!
Tests: 321, Assertions: 313, Failures: 18.

Your repos seem to be old and config changed.

  1. You have fuel/app/config/agent.php and browscap.url is out of dated. 'url' => 'http://browsers.garykeith.com/stream.asp?Lite_PHP_BrowsCapINI'
  2. In your fuel/app/config/config.php, base_url has been changed. 'base_url' => '/',
WanWizard commented 11 years ago

Which is exactly what I wrote earlier.

user00265 commented 11 years ago

Remove the file to pull defaults, or modify to fit what it should be. Does it work? doesn't here.

user00265 commented 11 years ago

My bad, hit the wrong button there.

WanWizard commented 11 years ago

Just ran the 1.5 tests:

Tests Running...This may take a few moments. PHPUnit 3.6.12 by Sebastian Bergmann.

Configuration read from /data/www/fuelphp/1.5/develop/fuel/core/phpunit.xml

............................................................... 63 / 314 ( 20%) ............................................................... 126 / 314 ( 40%) ............................................................... 189 / 314 ( 60%) ............................................................... 252 / 314 ( 80%) ..............................................................

Time: 4 seconds, Memory: 20.50Mb

OK (314 tests, 311 assertions)

So as far as I'm concerned, case closed. Let me know if you find the reason for them failing on your machine, so we can have a look at it.