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

integrate test suite #263

Closed
wants to merge 1 commit into from
Closed

integrate test suite #263

wants to merge 1 commit into from

Conversation

karussell
Copy link
Collaborator

@karussell karussell commented Oct 31, 2017

This PR fixes #23 still some minor todos - see below.

The test suite was taken from https://github.com/geocoders/geocoder-tester but using a simple java class to execute the tests to avoid setting up separate tooling. Most of the test cases that ran before already run and I was able to compare the current PR #254 with master:

test total request tests failures 254 (time it took) failures master (took)
Poland 120 50 (7s) 54 (18s)
USA 300 84 (20s) 81 (47s)
Germany 351 52 (20s) 50 (54s)
France1 145 77 (11s) 64 (30s)
France2 1810 932 (110s) 867 (300s)

So the quality of the new PR is really comparable, only in France or some things to improve. The performance comparison is a bit unfair as I've done photon vs. some other setup but some smaller tests with the same setup still indicate a fair speed up.

TODOs:

@yohanboniface
Copy link
Collaborator

The test suite was taken from https://github.com/geocoders/geocoder-tester but using a simple java class to execute the tests to avoid setting up separate tooling.

That's a bit sad, the geocoders tools are made so we can factorize the efforts in maintaining tools around geocoders them-selves.
I can fix any issue in geocoder-tester to make it easier to use, if needed.

@karussell
Copy link
Collaborator Author

karussell commented Oct 31, 2017

The problem is the sad state of python (why is python2 still around and not compatible with python3!?): I couldn't get python3 working as I need python2 for some other stuff here and a coexistence was above my linux foo. I can comment in the other repo if this helps.

Also I do think that we could share the data files and use different code to check this. For me as a Java dev I like having the different tests in my IDE and see kind of a summary and I also like that we could execute this on travis or somewhere else without much hassle.

@yohanboniface
Copy link
Collaborator

I couldn't get python3 working as I need python2 for some other stuff here and a coexistence was above my linux foo.

It's a matter of using virtualenvs. :)

I can comment in the other repo if this helps.

Yes, please. I can provide a complete recipe for non python devs.

@karussell
Copy link
Collaborator Author

It's a matter of using virtualenvs. :)

That was what I tried but couldn't get it working faster then writing some small Java code (which I also could slightly adapt to benchmark the GH API that currently has a custom JSON format).

yohanboniface added a commit to geocoders/geocoder-tester that referenced this pull request Oct 31, 2017
@yohanboniface
Copy link
Collaborator

See http://docs.python-guide.org/en/latest/dev/virtualenvs/

I can also setup a Docker image if that helps.

writing some small Java code

The first bits are always small and easy to setup. ;) As you known, the FLOSS hard bit is maintaining in the long term. :)

@karussell
Copy link
Collaborator Author

The first bits are always small and easy to setup. ;) As you known, the FLOSS hard bit is maintaining in the long term. :)

I know. But it is obvious that for others the test suite wasn't as accessible as it should be and so too much decoupling is also a risc :). So we should improve this, and as the test data is really the important bit here maybe we can download this on the fly and have it "directly integrated".

@karussell
Copy link
Collaborator Author

Another problem is if we wanted to run this regularly. Then we need to find a way to ignore certain failures but not all. Problem is mentioned in more detail here geocoders/geocoder-tester#20

@karussell
Copy link
Collaborator Author

Will try to avoid duplicating this here and enhancing the geocoder-tester

@karussell karussell closed this Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration external test cases
2 participants