fluent / fluent-logger-php

A structured logger for Fluentd (PHP)
Other
215 stars 57 forks source link

Added PHP 5.6, 5.5 and HHVM to travis.yml #25

Closed Nyholm closed 9 years ago

DQNEO commented 9 years ago

I tested it on my php 5.6 (OSX yosemite) and passed it. I hope this PR will be merged.

DQNEO commented 9 years ago

:+1:

repeatedly commented 9 years ago

@chobie How about this?

sasezaki commented 9 years ago

I think chobie nolonger has motivation for any PHP related project. Could someone merge this ?

repeatedly commented 9 years ago

HHVM always failed. Does someone fix this?

https://travis-ci.org/fluent/fluent-logger-php/jobs/54141689

DQNEO commented 9 years ago

I tried to reproduce the error on hhvm 3.6.0, but it makes another error.

[root@96dd6f494a62 fluent-logger-php]# git rev-parse HEAD
8f6532959d5f991e2734e537e40748c47efe755a
[root@96dd6f494a62 fluent-logger-php]# hhvm --version
HipHop VM 3.6.0 (rel)
Compiler: tags/HHVM-3.6.0-0-g6ef13f20da20993dc8bab9eb103f73568618d3e8
Repo schema: 3ab6576af394a33abbb29796c652f06b1c7640ce
[root@96dd6f494a62 fluent-logger-php]# hhvm vendor/bin/phpunit
PHPUnit 4.6.4 by Sebastian Bergmann and contributors.

Configuration read from /fluent/fluent-logger-php/phpunit.xml.dist

Deprecated configuration setting "strict" used

Fatal error: Class Mock_BaseLogger_189f6b0f contains abstract method (post) and must therefore be declared abstract or implement the remaining methods in /fluent/fluent-logger-php/vendor/phpunit/phpunit-mock-objects/src/Framework/MockObject/Generator.php(301) : eval()'d code on line 1

I am not familiar with HHVM , but I'll try.

repeatedly commented 9 years ago

Thanks! After fixed HHVM problem, I will merge this PR.

sasezaki commented 9 years ago

@DQNEO Fatal error: Class Mock_BaseLogger_189f6b0f contains abstract method (post) https://github.com/sebastianbergmann/phpunit-mock-objects/issues/223 seems same issue.

DQNEO commented 9 years ago

@sasezaki Thank you for your advice.

I created and uploaded a Docker Image so that anybody can run this project on HHVM https://registry.hub.docker.com/u/dqneo/hhvm/builds_history/182724/

thanks to #32 , I successfully reproduced the the same error as in travis-ci.

$ docker run  dqneo/hhvm:fluent
PHPUnit 4.6.4 by Sebastian Bergmann and contributors.

Configuration read from /opt/fluent-logger-php/phpunit.xml.dist

Deprecated configuration setting "strict" used

......F..Entity::__construct(): unexpected time format `not int` specified.
.......FMock_FluentLogger_14a93c89 could not write message debug.test: {"foo":"bar"}
.Mock_FluentLogger_14a93c89 connection aborted debug.test: {"foo":"bar"}
........................

Time: 2.19 seconds, Memory: 9.72Mb

There were 2 failures:

1) FluentTests\FluentLogger\ChainLoggerTest::testChains
Failed asserting that format description matches text.
--- Expected
+++ Actual
@@ @@
-["debug.test",%d,{"hello":"world"}]
+

/opt/fluent-logger-php/tests/Fluent/Logger/ChainLoggerTest.php:35

2) FluentTests\FluentLogger\FluentLoggerTest::testPostWillReturnFalseInTheCaseOfPostingUnsuccessfullyByReachedMaxRetryCount
Post method returned boolean
Failed asserting that true is false.

/opt/fluent-logger-php/tests/Fluent/Logger/FluentLoggerTest.php:59

FAILURES!
Tests: 42, Assertions: 55, Failures: 2.

Generating code coverage report in Clover XML format ... done

Generating code coverage report in HTML format ... done

So give me a chance again.

DQNEO commented 9 years ago

I invesigated why test fails on HHVM and have got the answer. Those 2 failures come from the same origin. It is a bug (or a strange behavior because there is no specification) of HHVM. I filed it to Facebook. https://github.com/facebook/hhvm/issues/5187

So, the best way I think is to eliminate HHVM on travis-ci test until they fix the bug.

I will make another PR.

repeatedly commented 9 years ago

I think allow_failures is good for this case:

http://blog.wyrihaximus.net/2013/12/getting-started-with-hhvm-testing-on-travis/

DQNEO commented 9 years ago

@repeatedly thank you for the information. I cannot modify this branch because it's not mine, so I committed here #34

Nyholm commented 9 years ago

Thank you @DQNEO for your work.

Thank you @repeatedly for merging.

repeatedly commented 9 years ago

np. Sorry for the late merging and thanks for your PR :+1:

DQNEO commented 9 years ago

:ok_woman: