-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
|
|
||
| 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?
| 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?
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.
This resets all parameters. This is admittedly also a deviation from the current behaviour. If you want to call the execution step again, you need to redo all steps that influence the osm2pgsql command line. We had exactly one test where this was an issue an it was doing the same update twice. So reuse looks to be a rare case and I'd rather err on the side of having to spell things out.
Windows can't handle the paths properly.
Windows needs external setup to function properly.
|
Next to addressing the comments, I've also changed the 'dict' matcher to a 'json' matcher as this might make more sense to most people (you can use it to match hstore fields beside the name) and added some tests to exercise the matcher functions (they weren't all used in our current tests). |
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.