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

Solve syntax error reporting and other minor improvements #101

Merged
merged 8 commits into from Apr 1, 2024

Conversation

mk-mxp
Copy link
Contributor

@mk-mxp mk-mxp commented Mar 30, 2024

Solves this forum discussion.

This replaces #97, which got no updates for a while. It is solving the problem, that syntax errors in students code fail in a way, that cannot be distinguished from failing tests, but tests are not reported by PHPUnit as failing.

I added a syntax linting step before invoking PHPUnit, which adds minimal overhead and only fails for hard syntactical errors.

While reading into the test runner specification I realized that we shouldn't write absolute pathes into the result messages. So I adjusted output to only have file names and no path at all. That fits well with the online editor only showing the file names on editor tabs, too.

I also improved the shell script and updated the README.

This is required, because PHPUnit 9.6+ no longer fails for syntax errors
with exit code 255. Instead, it converts the errors to exceptions and
handles them the same as every other exception (like the ones we use in
students stubs).

Use "all-fail" to show, that exceptions are now reported as individual
failing tests (for all tests here). This was based on a syntax error,
too. But that does not work like this test expected anymore.
This is recommended by test runner specification.
@mk-mxp mk-mxp requested a review from neenjaw as a code owner March 30, 2024 15:34
@mk-mxp mk-mxp requested review from ErikSchierboom and neenjaw and removed request for neenjaw March 30, 2024 15:35
@mk-mxp mk-mxp self-assigned this Apr 1, 2024
@mk-mxp mk-mxp added x:action/fix Fix an issue x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/test-runner Work on Test Runners x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises) x:size/large Large amount of work x:rep/large Large amount of reputation labels Apr 1, 2024
@mk-mxp mk-mxp merged commit 29ec7fb into exercism:main Apr 1, 2024
1 check passed
@mk-mxp mk-mxp deleted the solve-problems branch April 1, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/fix Fix an issue x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/test-runner Work on Test Runners x:rep/large Large amount of reputation x:size/large Large amount of work x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants