Skip to content
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

🚧 test generator #674

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

homersimpsons
Copy link
Contributor

@mk-mxp This is what I had in mind for a test-generator

Basically using twig templating gives a lot of powerful options to the developer.

The only important file here is test-generator/src/Application.php which is 126 lines and contains everything to generate an exercise from a template file. I just quickly hacked it so there are still improvements I put in test-generator/README.md (and certainly others I did not think of yet).

I find this approach far-easier to maintain (~150 lines of code) and to test (there is bascially nothing to test as this relies heavily on twig which is already tested).

I did not bother to parse the canonical-data.json file.

Copy link

github-actions bot commented Apr 1, 2024

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

@mk-mxp
Copy link
Contributor

mk-mxp commented Apr 2, 2024

Thanks for showing this PoC. I know Twig, so I have no doubt Twig is a way to make the test generator. In my eyes, putting all the logic into Twig simply moves it away from the language all our contributors should know best: PHP. So, no doubt, Application.php will be less complex. But the Twig templates must contain the complexity then. I agree, sprintf format strings don't look as nice as Twig does. But they have no dependencies and no special knowledge is required.

As I tried to explain in the README of my suggestion, it is not about the templating. The thing to ensure is, that all of the possible variations of canonical-data.json including the unknown / upcoming variations must be visible in the output of the test generator. That is the most relevant part of it.

The next relevant thing is: To ensure all output is thought about by the user of the generator. There is no use in a test generator that does what ChatGPT does: Simply put together code that somehow is running. The problem specifications are made for humans, and humans must interpret comments and values to form the test code accordingly. That is the core of Erik's warning about taking it to far: Do not think generating code means being done.

You are right, Twig is tested well. But does the Twig template actually do what is intended to do? That is, what I think tests are for. They ensure, that all the loops and variations behave as intended. So, no, testing cannot be skipped. One may use golden tests, which mean making up all the variations and comparing the results. But please, not only manually while developing, but runnable and repeatable.

That said, I would not mind combining the parsing of the canonical-data.json with Twig templates. It is required to separate out the known from the unknown parts and render all of those parts into some kind of template. There must be a way to force users to look into the code and not pretend they are done when there was no error thrown by the generator. How does that look within your suggestion?

Exercises to look at for the variations: list-ops is the most easy kind, bank-account has errors and multiple calls to different functions per test, complex-numbers has nested cases and relevant comments to show to the user (of the generator).

@mk-mxp
Copy link
Contributor

mk-mxp commented Apr 6, 2024

@homersimpsons There is a discussion going on about the test generation in the forum: https://forum.exercism.org/t/test-generators-for-tracks/10615

I think, this is what is different between my attempt and yours. You want to have an updating generator to produce production-ready test code. I want to have a first-time generator to generate code from scratch. These are 2 different goals and thus need 2 different tools.

AAA stands for Arrange-Act-Assert pattern
@mk-mxp mk-mxp mentioned this pull request Apr 21, 2024
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants