-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update Test Fixtures #82
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
…2 into bitner/update-test-fixtures
@eseglem I saw that you had done the work to update the examples in the ogc features repo. |
I am not sure I had thought about doing it that way. I believe I was comparing the strings directly to avoid involving the parser and any possible issues with it in this part of the tests. I had written some additional tests to run against the It would probably need to pull a specific commit hash to ensure the tests don't randomly break due to a change in their repo. And regularly updating that hash as long as nothing breaks. But I think that still seems like a nicer option than a copy. There is also a license mismatch which I was trying to avoid. This repo is MIT and theirs is not. I doubt it would cause any real issues, but nothing in this repo is directly copy and paste from theirs at this point to be safe about it. Perhaps a combination of the two is in order. Where the existing tests can be left as is and the tree comparison can be added to the other tests? Definitely open to your thoughts on it. Any concerns or issues with that? |
Yeah, I think if we left the existing tests as-is and added the ones against the ogc examples separately, it would be better to leave the existing ones as-is as just string tests. For the OGC examples case, though, I think we would want to keep the text tests exactly as they are in the OGC Features API repo in which case we need to do something like what I did. Looking at their license, if we just include the tests by manually copying (as in my PR here) we would need to add the copyright notice, but otherwise the license looks very compatible with MIT with no restrictions other than maintaining the copyright/license on copying/modifying things. I would say we could take two approaches to including those tests that would avoid that (by pulling directly, we would also always be pulling the copyright/license with the examples):
Git sub-module's can be confusing for a lot of folks when checking out repos, so I would probably lean towards that second option. |
Update the test fixtures with the latest examples from the ogc features cql2 examples.
For the test_json_to_text tests, rather than directly comparing the text output from str(model) on the json examples, round trip from json to model to text and back to the python model parsing the text to standardize representations. This may not be exact as directly comparing the strings, but allows for differences in case, whitespace, and double quotes.