Add option like skip-known-failures

[karussell]

See comment

[yohanboniface]

@karussell can you test with latest master? Known failing tests are now marked as "xfail" when in compare mode, so no traceback is reported. I think this is what you want (and no need for another option).

(Committed from the bus between the Granada airport and the city center \o/ )

[karussell]

Thanks! So all tests with xfail will be skipped when in compare mode or could they still PASS?

Update: Then we get XPASS and at the end we get a nice report that indicates that it will be still executed, makes sense but takes much longer.

[karussell]

Strange, something seems wrong:

================================================= 14 failed, 5799 passed, 1 skipped, 14548 xfailed, 29 xpassed in 2420.13 seconds ==================================================
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! NEW FAILURES !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
geocoder_tester/world/france/lorraine/test_addresses.csv::80 
geocoder_tester/world/france/lorraine/test_addresses.csv::23 
...
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! TOTAL NEW FAILURES: 14 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=================================================================================== NEW PASSING ====================================================================================
geocoder_tester/world/france/lorraine/test_addresses.csv::80
geocoder_tester/world/france/lorraine/test_addresses.csv::23
...

The log file used for comparison contains these entries:

...
geocoder_tester/world/france/lorraine/test_addresses.csv::80
...
geocoder_tester/world/france/lorraine/test_addresses.csv::23
...

[karussell]

When testing manually the new passing is correct but the new failure obviously not.

[karussell]

There seems to be another problem, while the tests it says:

geocoder_tester/world/poi/test_airports.csv::Dawadmi Domestic Airport XPASS

The summary says "4141 passed, 3451 xfailed, 20 xpassed" and "TOTAL NEW PASSING: 0" and "TOTAL NEW FAILURES: 0". Shouldn't the xpassed tests listed in the new passings section?

[yohanboniface]

Thanks, I'll have a look as soon as I'm in a bus or a plane (back from holidays tomorrow) :p Can you share the exact command line you are using?

[karussell]

I was just using save-reports and compare-reports nothing special:

py.test --api-url '/api?' --save-report report.log

[yohanboniface]

Update: Then we get XPASS and at the end we get a nice report that indicates that it will be still executed, makes sense but takes much longer.

Added --skip-xfail to cover this need. :)

Shouldn't the xpassed tests listed in the new passings section?

Definitely, and this is the behaviour I'm seeing, so something is different between our envs. I'll try to investigate more, but I may need help to be able to reproduce.

Strange, something seems wrong:

Failing to reproduce this too :(

[karussell]

Thanks a lot for the new parameter and investigating this. Will try to produce something reproducable for you in the next days!

[karussell]

Can I somehow send you both stored report logs and you can compare them? Or would you need one report log and a command to run the compare on demand to reproduce this?

[yohanboniface]

Can I somehow send you both stored report logs and you can compare them?

Let's try with that :) Can you put them in a pastthing somewhere?

[karussell]

Ok, finally managed to fix some other ugliness and have our master and the change up here: https://github.com/komoot/photon/issues/314#issuecomment-371528529

The results I have attached geocoder-tester.zip

[hbruch]

On import of compare-report, imported COMPARE_WITH lines are stripped. For queries starting/ending with whitespace, this results in having their failed tests reported as new failure and new passed, as they are compared to unstripped FAILED lines.

[karussell]

Ah, thanks for finding this. So should we adapt the test cases or improve the code?

[karussell]

(or both :))

[hbruch]

Oh, just thinking that other geocoders could make a difference of a number followed by a whitespace or not: in the first case, this could be a complete housenumber, in the second an unfinished postal code. So better just fix the code...

[yohanboniface]

Good catch @hbruch! Just pushed a fix. :)

[hbruch]

Thank you for the immediate fix! May I suggest to remove the linefeed via rstrip('\r\n') (for folks using windows ;-) ?