Repository navigation
Add test generator - #380
Conversation
The generator is implemented in Python and modelled on the x86-64-assembly track's generator. It keeps the stub in source/*.d and replaces everything from the `unittest` line onwards. It uses `.meta/additional-test-cases.json` if the file exists. Generators are added for - all-your-base - game-of-life - isbn-verifier - largest-series-product - matrix - phone-number The generated tests are identical to the existing tests. Python style is enforced by ruff, run by pre-commit and a new pre-commit workflow. Upgrade workflows to ubuntu-26.04. Sync .gitignore from generic-track. I used Claude for this PR.
|
Hello 👋 Thanks for your PR. This repo does not currently have dedicated maintainers. Our cross-track maintainers team will attempt to review and merge your PR, but it will likely take longer for your PR to be reviewed. If you enjoy contributing to Exercism and have a track-record of doing so successfully, you might like to become an Exercism maintainer for this track. Please feel free to ask any questions, or chat to us about anything to do with this PR or the reviewing process on the Exercism forum. (cc @exercism/cross-track-maintainers) |
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 | ||
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 | ||
| with: | ||
| python-version: "3.12" |
There was a problem hiding this comment.
Is this the default on Ubuntu 26? It seems old.
There was a problem hiding this comment.
The first draft of the file had Ubuntu 24. Yes it is the default there (Python 3.12.3).
(My desktop is also Ubuntu 24 with Python 3.12.3.)
| with: | ||
| python-version: "3.12" | ||
| - name: Set PY | ||
| run: echo "PY=$(python -VV | sha256sum | cut -d' ' -f1)" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
What's the purpose of the hash here? Is the Python version set by hah and not version string?
There was a problem hiding this comment.
This was in ahans' exercism/arm64-assembly#158 and I had never looked into it. Turns out it is a key for the cache, but we can remove these lines.
| - Create the test suite in $(jq -r '.test' <<< "${files}") | ||
| - The tests should be based on the canonical data at 'https://github.com/exercism/problem-specifications/blob/main/exercises/${slug}/canonical-data.json' | ||
| - Any test cases you don't implement, mark them in 'exercises/practice/${slug}/.meta/tests.toml' with "include = false" | ||
| - The tests can be generated by adding 'generators/exercises/${slug//-/_}.py' and running 'bin/generate ${slug}' |
There was a problem hiding this comment.
Would it make more sense to move this into .meta?
| # Example generating every exercise that has a generator: | ||
| # bin/generate --all | ||
|
|
||
| set -euo pipefail |
There was a problem hiding this comment.
Does this wrapper add any value?
There was a problem hiding this comment.
Removed.
Lean's wrapper does add value. I never gained muscle memory without it.
Yesterday it would have been nice to have
bin/configlet sync -u -e list-ops
bin/generate list-ops
bin/verify-exercises-in-docker list-opsin every track.
| def generated_slugs() -> list[str]: | ||
| return sorted( | ||
| path.stem.replace("_", "-") | ||
| for path in GENERATORS.glob("*.py") |
There was a problem hiding this comment.
Wouldn't the filename match the exercise name?
| for slug in slugs: | ||
| try: | ||
| generate(slug, specifications) | ||
| except GeneratorError as error: |
There was a problem hiding this comment.
Should the generator continue on error?
There was a problem hiding this comment.
We now stop.
If a file was missing or the exercise stub was empty, we print an error message.
If an exception occurs in an exercise .meta/slug.py, it presumably won't be any of FileNotFoundError, EOFError, subprocess.CalledProcessError. We don't catch the error, and Python prints a stack trace.
| for char in value: | ||
| if char in _ESCAPES: | ||
| chars.append(_ESCAPES[char]) | ||
| elif ord(char) < 0x20 or ord(char) == 0x7F: |
There was a problem hiding this comment.
Can this use the printable test?
|
|
||
| def d_int(value: int) -> str: | ||
| """Render an integer as a D integer literal.""" | ||
| if isinstance(value, bool) or not isinstance(value, int): |
There was a problem hiding this comment.
Why is this test needed? The string function doesn't do type validation
| rev: v6.0.0 | ||
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer |
There was a problem hiding this comment.
Some of these checks (especially end-of-file) would be good to use widely. These might be nice to add to the generic-track* repos :D
There was a problem hiding this comment.
My impression was that the generic-* repos essentially never change, but now I notice you have merged to generic-test-runner every few months. Should I have created a PR when I noticed https://forum.exercism.org/t/dependabot-frequency/53725 ?
There was a problem hiding this comment.
Yes please. We clone those for each new track. If we keep those up to date and shiny, new tracks start off with the latest and greatest tooling we have!
| @@ -0,0 +1,14 @@ | |||
| from lib import assert_eq, d_array, d_inline_array | |||
There was a problem hiding this comment.
- Out of curiosity, is there a reason you didn't want to use templates?
- If these files live in .meta, would it make sense for them to use a standard filename, eg
test_generator.py, similar to the other meta files?
There was a problem hiding this comment.
-
I have never been a big user of templates, for example my first web development was with Java servlets and I never bothered with JSP. I can read and write without templates more easily than with templates
-
Now using
test_generator.py
| def flatten(cases: list[Case], parents: tuple[str, ...] = ()) -> list[Case]: | ||
| """Flatten nested groups, recording the descriptions of enclosing groups.""" | ||
| flattened = [] | ||
| for case in cases: | ||
| if "cases" in case: | ||
| description = case.get("description") | ||
| nested = (*parents, description) if description else parents | ||
| flattened.extend(flatten(case["cases"], nested)) |
There was a problem hiding this comment.
| def flatten(cases: list[Case], parents: tuple[str, ...] = ()) -> list[Case]: | |
| """Flatten nested groups, recording the descriptions of enclosing groups.""" | |
| flattened = [] | |
| for case in cases: | |
| if "cases" in case: | |
| description = case.get("description") | |
| nested = (*parents, description) if description else parents | |
| flattened.extend(flatten(case["cases"], nested)) | |
| def flatten(cases: list[Case], parents: list[str] | None = None) -> list[Case]: | |
| """Flatten nested groups, recording the descriptions of enclosing groups.""" | |
| flattened = [] | |
| if parents is None: | |
| parents = [] | |
| for case in cases: | |
| if "cases" in case: | |
| description = case.get("description") | |
| nested = parents + [description] if description else parents | |
| flattened.extend(flatten(case["cases"], nested)) |
| # Avoid leaving __pycache__ in the exercise directory. | ||
| sys.dont_write_bytecode = True |
There was a problem hiding this comment.
This should probably either go at the top of the file or, if you want to localize the change, be reset.
| raise EOFError(f"{path.relative_to(ROOT)} is empty; create the stub first") | ||
| match = re.search(r"^unittest\b", content, flags=re.MULTILINE) | ||
| if match: | ||
| return content[: match.start()] |
There was a problem hiding this comment.
| return content[: match.start()] | |
| return content[:match.start()] |
Make spacing consistent with describe()
There was a problem hiding this comment.
ruff format demands that space and puts it back.
There was a problem hiding this comment.
But it's fine with description[:1].upper() on line 114? That's odd. But probably not worth fighting ruff over.
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", |
There was a problem hiding this comment.
I suspect there's no NULL or CR chars in any of the exercises, but I suppose it doesn't hurt to have these.
| rev: v6.0.0 | ||
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer |
There was a problem hiding this comment.
Yes please. We clone those for each new track. If we keep those up to date and shiny, new tracks start off with the latest and greatest tooling we have!
| rev: v0.16.9 | ||
| hooks: | ||
| - id: ruff-check | ||
| args: [--fix, --target-version, py314] |
There was a problem hiding this comment.
I'm a bit surprised ruff doesn't pick up the system version 🤷♂️ We could also drop a .ruff.toml config file to move the version from the workflow file to the repo "main" content.
There was a problem hiding this comment.
Adding a .ruff.toml config file
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
This PR has been translated 🚀 |
The generator is implemented in Python and modelled on the x86-64-assembly track's generator.
It keeps the stub in source/*.d and replaces everything from the
unittestline onwards.It uses
.meta/additional-test-cases.jsonif the file exists.Generators are added for
The generated tests are identical to the existing tests.
Python style is enforced by ruff, run by pre-commit and a new pre-commit workflow.
Upgrade workflows to ubuntu-26.04.
Sync .gitignore from generic-track.
I used Claude for this PR.