Code review issue - make check

[XavierAntoviaque]

During the last code review I've built the "make check" command, where I've put tests for some of the issues I saw. This ticket is just a quick reminder to run it and fix the issues it shows until the script is happy : )

[XavierAntoviaque (xavier-antoviaque)]

Just fixed the OWA errors from make phpcheck - but there are still some errors coming from hackit (cf attached file)

[XavierAntoviaque (xavier-antoviaque)]

Just ran it on both my local installation and hackit.cx, and I get different results... :/

Vlad, David, could you attach the file tests/php/simpletest.log generated by make phpcheck, to compare? thx!

I'm attaching the one generated by hackit.cx in this post, the previous message contained the one from my local installation.

[Vlad Dragu (vlad-dragu)]

See attached my simpletest.log
The weird thing is that the only failure i get, i get it only when i run make phpcheck. If i run the tests in the browser i do not get the error.
The 4 notices will be easy to fix.
David, most of the warnings you get are caused by the fact that youre're probably missing the DEFAULT_TIMEZONE definition from your config.php file. please copy it from config.php.dist which is in the repository along with the lne below it.
From what i see my log resembles much with David's log
I'll continue to check it and make comparisons and see what i came up with

[XavierAntoviaque (xavier-antoviaque)]

Hmm - the difference could come from differences between the cli php.ini and the apache one. Could you post both? I'll try to investigate this a bit more on my side.

[Vlad Dragu (vlad-dragu)]

This is my cli php.ini file

[Vlad Dragu (vlad-dragu)]

Here is my apache php.ini

[Vlad Dragu (vlad-dragu)]

I fixed the warnings that i still saw in my simpletest.log and also fixed that fail message. It was not caused by a difference in php.inis, i just had to change the way i reset the game_session singleton. Right now, i get no errors from make phpcheck.
I'm attaching the simpletest.log

[XavierAntoviaque (xavier-antoviaque)]

I think it comes from the difference for the error_reporting variable. Your php.ini is set to not display E_NOTICE, so if you ran the tests on a version that didn't contain the call to error_reporting(E_ALL & ~E_DEPRECATED); in tests/php/hackit_all_tests.php, this would explain why you don't see any error.

I've attached my latest simpletest.log, with E_NOTICE reporting enabled.

[XavierAntoviaque (xavier-antoviaque)]

Btw, remember that this bug isn't only about make phpcheck, but about make check, which doesn't run successfully.

[Vlad Dragu (vlad-dragu)]

my setting in the cli php.ini for error reporting was E_ALL & ~E_DEPRECATED this setting means that it shows the notices also.
I also tried with just E_ALL and i only got the deprecated errors
Please tell me what setting do you use in your cli php.ini
Also i tried to do a make check and it gives me the error that "executable files have been found". the files listed are almost all files form the jscoverage directory, the bzr directory, the classes directory and the config directry. Please tell me what exactly this error reffers to.

[XavierAntoviaque (xavier-antoviaque)]

In my php.ini I have E_ALL, but this is superseeded by the error_reporting() call that I added to the test suite. Actually, I have seen that you have removed it - what was the issue with it?

For the executable files, it means that some files had the execution bit set (the --x in the rights shown by ls), which should only be the case for scripts or binaries that are meant to be executed from the command-line. For php or JS scripts that are read by apache, the rights should be 644 (cf http://catcode.com/teachmod/ ).

This is a common issue for files that are read on a windows filesystem, since it doesn't support the unix convention for file rights.

[Vlad Dragu (vlad-dragu)]

I removed the error_reporting() call because i didn't wanted to interfere with the settings in the ini files both cli and apache.
Sometimes setting values at runtime can have unexpected results and i wanted to remove that complexity altogether.
My goal was to try to get to a setting that will output the same warnings that i see in your log file and work from there to remove them.
However, no matter what settings i use i cannot see those notices.
The warnings are still there for you, right?

I'll work on the file permissions and then run make check again

[XavierAntoviaque (xavier-antoviaque)]

Yes, I just ran it again, I can still see the issues. :/ There are different on my local machine and on hackit.cx, but this is probably because of the difference of version of PHP (cf attached).

I tried (cf attachment):
* to run a diff between our two php.ini files, and there was no other difference...
* to see if the error_reporting isn't reset somewhere else - it isn't.
* to print the versions of PHP (different in local laptop and on hackit.cx, but both show errors)

Could you try to run the same commands (cf attachment) and copy it here?

And as a last resort if everything else fails, I've also attached the list of files on hackit.cx and on my local laptop, each with its md5 - if you can run this command and also post it here, I'll try to see if there are discrepancies (run it with the trunk version from today to avoid getting daily changes in the way - I will avoid pushing to it until you can do this):

$ find -type f -exec md5sum {} \; > md5_vlad.log

[Vlad Dragu (vlad-dragu)]

This is my command line output.

[Vlad Dragu (vlad-dragu)]

This is md5 sum file

[Vlad Dragu (vlad-dragu)]

I removed all errors that i got when running make check.
Now it gives me a success message.
The difference between phpmakecheck on different systems still stands

[XavierAntoviaque (xavier-antoviaque)]

Here is the log of the chatroom at http://community.hackit.cx/chat/ for the 04/02/2011.

02:29:12 <da> Xavier quit (timeout)
07:55:30 <da> Xavier quit (timeout)
08:53:11 <> vlad joins Farsides
08:57:35 <Xavier> hi Vlad!
08:57:47 <vlad> Hi Xavier
08:58:36 <Xavier> Congratz for finishing the work on admin views yesterday - I didn't merge it yet, but I can't wait to see what it gives :D
09:00:09 <vlad> thanks :) yup, it went well :)
09:00:30 <Xavier> : )
09:01:10 <Xavier> I'll let you settle before discussing today - let me know when you had time to drink your coffee :)
09:02:02 <vlad> i'm good, just give me 5 mins to start the debian install. i'll exit this chat and login again from there
09:02:10 <Xavier> ok
09:05:20 <da> vlad quit (timeout)
09:08:36 <> vlad joins Farsides
09:08:56 <vlad> ok, i'm back. how do we aproach this?
09:12:50 <Xavier> Good question - the first setps depend on me, I'll need to merge the latest code (probably on a new beta instance to be able to work on the latest codebase)
09:13:06 <Xavier> To see if the issue is still there
09:13:34 <Xavier> maybe we could try to have the same version of PHP, too - what do you have on your debian?
09:14:07 <Xavier> I have 5.3.3-0.dotdeb.1 on hpo.com
09:14:29 <vlad> i have 5.3.2-2
09:16:13 <Xavier> We could both upgrade to the latest version on php53.dotdeb.org, which is 5.3.5-0.dotdeb.0
09:16:34 <Xavier> At least we would know this doesn't come from a version difference - what do you think?
09:16:51 <vlad> yup. let's do that
09:20:10 <> Xavier joins Farsides
09:20:36 <Xavier> done - the game seems to be still running, which is good already ;p
09:20:52 <Xavier> I'll start installing the beta instance
09:55:11 <> vlad joins Farsides
09:55:21 <vlad> i'm having trouble upgrading. seems i need libicu38 > 3.8.5 and it's not installable and the only version i find on the net is 3.8.1 any ideas?
09:56:07 <> Xavier joins Farsides
09:56:31 <Xavier> uh, it seems I was disconnected - I was writing to you without you seeing it : )
09:56:45 <Xavier> let me see for the dependency
09:57:36 <vlad> ok, thanks
09:58:35 <Xavier> weird, I have 3.8.1-3+lenny2 and it installed without issue
09:58:55 <Xavier> what do you have in your apt.sources?
09:59:26 <vlad> deb http://http.us.debian.org/debian squeeze main contrib non-free deb http://ppa.launchpad.net/bzr/ppa/ubuntu jaunty main deb-src http://ppa.launchpad.net/bzr/ppa/ubuntu jaunty main deb http://security.debian.org/ squeeze/updates main deb-src http://security.debian.org/ squeeze/updates main deb http://packages.dotdeb.org stable all deb-src http://packages.dotdeb.org stable all deb http://php5
10:00:58 <Xavier> ah, you're in testing... It's likely that one of the packages in squeeze has a dependency that is newer than the packages in stable
10:00:59 <vlad> deb http://php53.dotdeb.org stable all deb-src http://php53.dotdeb.org stable all
10:01:34 <vlad> ok, how do i fix it? :)
10:02:01 <Xavier> I'm thinking - it can come from a variety of reasons : )
10:03:01 <vlad> hmm actually it seems i need 3.8-5 can this be smaller than 3.8.1-3?
10:03:43 <Xavier> nope, it's higher : )
10:03:56 <Xavier> try http://www.dotdeb.org/2011/01/11/php-5-3-5-now-for-s...