-
-
Notifications
You must be signed in to change notification settings - Fork 479
Add script for style testing #2436
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
base: master
Are you sure you want to change the base?
Conversation
Windows can't handle the paths properly.
Windows needs external setup to function properly.
| Per default, behave assumes that the build directory is under `osm2pgsql/build`. | ||
| If your setup works like that, you can leave out the `-D` parameter. | ||
|
|
||
| To make this a bit easier a shell script `run-behave` is provided in your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand this correctly, run-behave is not installed any more, but still mentioned here.
|
|
||
| def __call__(self, base_url): | ||
| assert base_url == self.expected_base_url,\ | ||
| f"Wrong replication service called. Expected '{self.expected_base_url}', got '{base_url}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line continuation formatting with backslash is inconsistent in whole file: sometimes with, somtimes without space in front.
Also: There are some line continuations after a comma which should not be needed?
|
|
||
| if output is not None: | ||
| context.osm2pgsql_params['-O'] = output.strip() | ||
| if output == ' pgsql' and 'S' not in context.osm2pgsql_params: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this bis '-S' not in?
| context.osm2pgsql_cmdline = ' '.join(cmdline) | ||
| context.osm2pgsql_outdata = [d.decode('utf-8').replace('\\n', '\n') for d in outdata] | ||
| context.osm2pgsql_returncode = proc.returncode | ||
| context.osm2pgsql_params = {'-d': context.user_args.test_db} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this set here after building the command line in line 503 and 507?
This adds the script osm2pgsql-test-style which allows blackbox testing of osm2pgsql lua styles. The script uses the same BDD test style as already introduced with osm2pgsql's integration tests. For details and documentation see: osm2pgsql-dev/osm2pgsql-website#78
The PR also changes the internal tests to use the style tester. The semantics of the steps have slightly changed to make them more consistent and understandable. So there were some changes to the tests necessary, most notably:
ST_AsText()but use the!geomatcher instead.Note: the tester originally also changed the behaviour with respect to environment variables and ignored the ones set outside. However, it turns out that
pg_virtualenvneeds somePG*variables set to have osm2pgsql use the right cluster. And the Windows CI just refused to do such basic things like name resolution when some of its variables are unset. I've not further investigated but simply reinstated the previous behaviour of forwarding all environment variables.