Fixed broken Filter::XSSFilter() unit tests

[kristuff]

Currently, testXSSFilterWithNonStringArguments() method in tests\core\FilterTest.php does not test the XSSFilter() method. Changed from:

public function testXSSFilterWithNonStringArguments()
{
   $this->assertEquals(123, 123);
   $this->assertEquals(array(1, 2, 3), array(1, 2, 3));
   $this->assertEquals(17.001, 17.001);
   $this->assertEquals(null, null);
}

to:

public function testXSSFilterWithNonStringArguments()
{
    $this->assertEquals(123, Filter::XSSFilter(123));
    $this->assertEquals(array(1, 2, 3), Filter::XSSFilter(array(1, 2, 3)));
    $this->assertEquals(17.001, Filter::XSSFilter(17.001));
    $this->assertEquals(null, Filter::XSSFilter(null));
}

[panique]

Excellent man, excellent It's so weird that these tests did basically nothing :)

[kristuff]

Yep ;) But unfortunately, the test does not pass. I will investigate.

[panique]

Aaah i think i found the bug :) The last test uses null as parameter, which will not work in this case, as null cannot be handled as a variable inside the function itself. Can you just remove that line that tests with null ? Big Thanks for the tests, i'll then merge instantly!

[kristuff]

For my first contribution here, sorry to inconvenience :)

Sorry, I could remove the using of null but it won't change anlything. I already made all test combinaisons.

In fact, you also noted the tests also failed with PHP 5.6., and be carrefull the raison they pass with PHP7.

I am a quit new to PHPunit and travis (and even Github). After some tests in a dummy project https://github.com/Kristuff/dummyPHP, I have reproduced that error with all PHP versions (5.3 to 7.1 and hhvm) and fixed it by changing the test method like in last commit. Using of dedicaded method like assertNull instead assertEquals($null, ...) is optional but the recommanded way, it's why I changed it (the two ways work like a charm in my test project).

The second issue (still not fixed) is the PHPUnit version used on travis.
The PHPUnit version running in my test project is 5.7.19 on PHP 5.6 when it is 5.7.15 here (I forced install and run of last stable PHPUnit)

And concerning PHP 7 and 7.1. I already seen the issue. Travis currently uses the default version of PHPUnit compatible with PHP7, and it's now 6.0 . Tests class are not compatibles with the new namespace synthax. The log says 'No tests executed!' and it's why the tests pass!

Note that it is also the case for all other tests of this project (I check another recent pull request). Currently, there is no tests executed of PHP7 at all.

I think we need to force travis using the last compatible PHPUnit version (5.7.* which is supported on PHP 5.6, 7.0, 7.1). I will try to make the change soon.

And by the way, thank you very much for this project. I learned a lot with it :)

[kristuff]

Great, it finally pass! :)

[panique]

Thanks! :) I've removed the weird StyleCI syntax check as it had strange results

[kristuff]

Hi,
I see an attempt to send code coverage report to Scrutinizer in travis.yml.

# gets tools from Scrutinizer, uploads unit tests results to Scrutinizer (?)
...

The code coverage report is not generated for now because there is no white list defined.

I have configured the white list in phpunit.xml configuration like this:

 <filter>
        <whitelist processUncoveredFilesFromWhitelist="true">
            <directory suffix=".php">../application/core</directory>
            <directory suffix=".php">../application/model</directory>
            <directory suffix=".php">../application/controller</directory>
        </whitelist>
    </filter>

You can see result here https://travis-ci.org/Kristuff/huge/jobs/225366327
Not really sure about what should be include in the list. Let me know if you want make the change.

[panique]

Thanks a lot, awesome! I'm adding this to the project

[kristuff]

Your are welcome. To close the XssFilter chapter (I hope :)... ), I see an open issue #790 and a blocked PR #794 with [TODO] concerning a recursive implementation of XssFilter. I have implemented and fully tested the solution proposed here #790 (the method accepts now string, array and object).

I can try to make another PR? My tests passed here https://travis-ci.org/Kristuff/huge/builds/225608242

[panique]

Awesome!! Can you please make a PR, this would be super cool! Thanks a lot @kristuff

[kristuff]

done :)