🧹 Reduce duplication in snippet testing #5345
Conversation
for more information, see https://pre-commit.ci
…nne/hedy into reduce-snippet-testing-duplication
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…nne/hedy into reduce-snippet-testing-duplication
for more information, see https://pre-commit.ci
…nne/hedy into reduce-snippet-testing-duplication
rix0rrr
changed the title
|
|
||
|
|
||
| def filter_snippets(snippets, level=None, lang=None): | ||
| if (lang or level) and os.getenv('CI'): | ||
| raise RuntimeError('Whoops, it looks like you left a snippet filter in!') | ||
|
|
||
| if lang: | ||
| snippets = [(name, snippet) for (name, snippet) in snippets if snippet.language[:2] == lang] | ||
| snippets = [s for s in snippets if x.language[:len(lang)] == lang] |
There was a problem hiding this comment.
x.language should be s.language in this line and the following
There was a problem hiding this comment.
Tiens. I thought pylint would catch errors like these.
| # English file is always 'en.yaml' in the same dir | ||
| en_file = path.join(path.dirname(snippet.filename), 'en.yaml') | ||
|
|
||
| # Read English yaml file | ||
| original_yaml = YamlFile.for_file(en_file) | ||
| original_loc = yaml_locator(snippet, original_yaml) | ||
| original_loc = locate_snippet_in_yaml(original_yaml, snippet, yaml_locator) |
There was a problem hiding this comment.
The yaml_locator does not need to be supplied any more
| @@ -9,18 +9,17 @@ | |||
| - Snippets can hold on to the information where in the YAML file they were | |||
There was a problem hiding this comment.
This could be scratched now, right :)
|
Good catches @boryanagoncharenko, thanks! I'm a bit surprised that the static analysis tools we run didn't catch these, since they're pretty obvious. I've confirmed that We should probably find a static checker to catch trivial errors? |
|
I've re-added We used to have it, not sure why we removed it? |
|
Doesn't seem to be easy to re-enable. My struggles will continue in here: #5377 In the mean time, this one should be good to go. |
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
The snippet testing code had some duplication. In this code, we separate out how to find the interesting snippets in the YAML files. For every snippet, we remember where in a YAML file we found it (by keeping track of the keys we used to descend into dictionaries and the indexes we used to descend into lists).
That way, the "restore to English" code can become generic: since every snippet knows where in the YAML structure it comes from, we can automatically find the same snippet in the English language file and copy it over.
How to test
If the tests pass, everything works!