Refactor json_test's TEST_OBJECTS value into test data files #2527
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpharrigan
approved these changes
Nov 12, 2019
…the name - Include keys from json.py in "expect test data to be present" check
mpharrigan
reviewed
Nov 13, 2019
| @@ -667,6 +518,88 @@ def test_to_from_strings(): | |||
| cirq.read_json(io.StringIO(), json_text=x_json_text) | |||
|
|
|||
|
|
|||
| def _eval_repr_data_file(path: pathlib.Path): | |||
| return eval(path.read_text(), { | |||
There was a problem hiding this comment.
this seems mildly dangerous
There was a problem hiding this comment.
I agree in principle... but to be pragmatic we're already parsing files and running them as python when we invoke pytest. I don't see any additional risk from this... if there was, it would be of the style "a tool was setup to detect people replacing code with delete everything commands, but the tool ignored things that didn't end with .py".
There was a problem hiding this comment.
yup, logically this isn't much different than the gaping security hole of python / pytest / everything already :)
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A status check is failing. |
CirqBot
removed
automerge
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This spreads out the gigantic list a bit, but mainly it allows us a format for adding backwards compatibility tests (where the repr file contains the intended result, instead of the object that initially produced the JSON).
The test files were generated by iterating over the test object and using cirq.to_json and repr.
Fixes #2425
This is maybe the third time I find myself refactoring this code. I wonder what the next one will be?