-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
New test runner design #130
Conversation
- Have a Tracer class to track test runs - Extract required data from Passed events - Produce JSON from Result objects - Inject the output file name by environment variable, use fallback name for local runs
This only changes the test names from method names to what PHPUnit sees as prettified test name.
- Use PHPUnit from vendor/* - Provide env var with result file path - Strip out no longer required log files and post-processor The Dockerfile must be written further. composer dependencies should be installed in build step and only "vendor/*" transfered to the runtime image. bin/run.sh still contains debug output.
We don't have to disallow output on the command line anymore, the event fires always.
Also must fail total result on one failed test in list
This must have the absolute exercise path, but cannot get it from PHPUnit. So a new env var provides this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for this implementation! I find it way less hacky than the previous one. It also removes a lot of code which means it will be easier to maintain.
Thank you for improving the Dockerfile too. This would probably improve the execution time of the test runner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all those improvements!
Note: I let some threads open, but they are hidden, I think it would be great to make those changes, but if you don't want to feel free to skip those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM I'll let the PHP-specific review for the other reviewers
Thanks to everyone! I'll merge on Thursday, as I have more time that day in case of unexpected things happening. |
As suggested in #110 a re-implementation of all current use cases (test cases) using PHPUnit events. This reduces the complexity a lot.
@exercism/maintainers-admin I changed code ownership, as this replaces the existing test-runner completely.
@homersimpsons Do you want to take a look, also?
I also optimized the files remaining in the Docker image: