Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option like skip-known-failures #37

Open
karussell opened this issue Jan 22, 2018 · 18 comments
Open

Add option like skip-known-failures #37

karussell opened this issue Jan 22, 2018 · 18 comments

Comments

@karussell
Copy link
Contributor

karussell commented Jan 22, 2018

See comment

@yohanboniface
Copy link
Member

@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
Copy link
Contributor Author

karussell commented Feb 24, 2018

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
Copy link
Contributor Author

karussell commented Feb 25, 2018

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
Copy link
Contributor Author

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

@karussell
Copy link
Contributor Author

karussell commented Feb 25, 2018

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
Copy link
Member

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
Copy link
Contributor Author

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

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

yohanboniface added a commit that referenced this issue Feb 28, 2018
@yohanboniface
Copy link
Member

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
Copy link
Contributor Author

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

@karussell
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

Ok, finally managed to fix some other ugliness and have our master and the change up here: komoot/photon#314 (comment)

The results I have attached
geocoder-tester.zip

@hbruch
Copy link
Contributor

hbruch commented Mar 9, 2018

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
Copy link
Contributor Author

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

@karussell
Copy link
Contributor Author

(or both :))

@hbruch
Copy link
Contributor

hbruch commented Mar 9, 2018

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
Copy link
Member

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

@hbruch
Copy link
Contributor

hbruch commented Mar 9, 2018

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

yohanboniface added a commit that referenced this issue Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants