Update the PHP library with new internal domain tests about referer-parser HOT 8 CLOSED

To clarify the last point: @fblundun disabled the sync_data.py sync for PHP in case the new referer-tests.json format broke the PHP test suite

from referer-parser.

😕 I am confused. What do I need to do?

from referer-parser.

Hi @lstrojny - sorry for the confusion. Firstly, this ticket can't be done until branch feature/json-tests has been merged into master.

Looking at your tests I can see you are already testing the custom internal hosts functionality here: https://github.com/snowplow/referer-parser/blob/master/php/tests/Snowplow/RefererParser/Tests/AbstractParserTest.php#L67

The idea of the two new tests in the master copy of referer-tests.json is that they will only work if the parser has been configured with the internal hosts "www.subdomain1.snowplowanalytics.com" and "www.subdomain2.snowplowanalytics.com" - otherwise the source will incorrectly be "unknown" rather than "internal".

The versions of the Referer-Parser which have not implemented a custom list of internal hosts will not be able to successfully run the new tests. So I have edited sync_data.py so that it only copies referer-tests.json into the java-scala version. The idea is that whenever another language library implements custom lists of internal hosts, sync_data.py will be updated so that the master copy of referer-tests.json gets copied into that library's directory.

So when the new branch gets merged, you should:

• manually copy the new version of referer-parser/resources/referer-tests.json into referer-parser/php/tests/Snowplow/RefererParser/Tests/referer-tests.json
• edit AbstractParserTest.php so that the parser gets created with "www.subdomain1.snowplowanalytics.com" and "www.subdomain2.snowplowanalytics.com" in its list of internal domains so that the new tests in referer-tests.json pass
• edit sync_data.py so that automatically copies referer-parser/resources/referer-tests.json into referer-parser/php/tests/Snowplow/RefererParser/Tests/referer-tests.json

Does that make sense?

from referer-parser.

A lot, thank you! Can you ping me here again once the branch is merged?

from referer-parser.

Great! I'll do that.

from referer-parser.

@lstrojny - the branch has been merged so you should be good to go! Let me know if you need anything clarified.

from referer-parser.

Thanks for the ping, I'll have a look.

from referer-parser.

Done in 2ac5175 and c226e21

from referer-parser.