fix: clarify Python pytest failure messages - #24
Conversation
Keep custom assertion messages intact and retain default pytest diagnostics for failed Python tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve the upstream Python error-handling contract while retaining detailed pytest failure diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves Python pytest result parsing so learners receive more constructive, non-truncated failure feedback (custom assertion messages and full E ... diagnostic lines), aligning with Issue #23’s request for clearer outcome messaging.
Changes:
- Reworked
parsePytestOutputto extract per-failure error message blocks (including multiline assertion details) and distinguish failures vs errors for counting. - Added dependency-free Node regression tests covering custom assertion messages, default assertion diffs, exceptions, multiple failures, collection errors, and mixed failure/error runs.
- Updated
npm testto run the new regression suite.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| executor.js | Updates pytest output parsing to preserve detailed assertion/exception diagnostics and count errors alongside failures. |
| test/executor.test.js | Adds Node-based regression tests for pytest parsing behavior across failure/error scenarios. |
| package.json | Wires npm test to run the new regression tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ensure production requests complete assertion diagnostics through a tested invocation contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise real verbose pytest output and preserve response-state behavior across success, failures, collection errors, no tests, and process failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
test/pytest-integration.test.js:12
npm testnow runstest:integration, which unconditionally spawns thepytestbinary. Ifpytestis not installed / not on PATH (common outside the Docker image),spawnSyncwill return an error and this test will fail (or produce confusing assertions) even though the Node code is fine. Consider skipping this integration regression whenpytestis unavailable, or failing with an explicit message.
const versionResult = spawnSync('pytest', ['--version'], { encoding: 'utf8' });
const pytestVersion = (versionResult.stdout || versionResult.stderr).trim();
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Follow-up plan: preserve multiline failure formatting in codewit.usPR #24 correctly returns the complete
Acceptance criteria
This should be implemented as a small follow-up PR in |
|
@DAC098 I'm curious about your thoughts on multi-line support once you have some time testing this out with real exercises. I'm tempted to push the multi-line output to later (or not planned) because I suspect most of our failure messages should be more simplistic and not depend on formatting across multiple lines. I can imagine some situations where that might not be the case, but shipping this feature soon is a higher priority. |
|
I could see having multi-line messages being useful for more complicated tests but should probably keep it to simple messages as much as possible. if the message is complicated enough then maybe we could make notes or hints about possible errors that could happen in the prompt of the exercise. I am going to look more into the PR and test it against the exercises that I updated since I can just directly send tests to codeval for quicker results. I was curious if pytest supported doing like a json output (or structured output of any kind) to see if we could reduce the amount of stuff that we would need to parse. pytest does support doing |
Good points. Let's wrap up this PR without those features, but I'll note both Multi-line messages and possible json results as potential future features. |
|
I have been looking through the PR, once of the exercises I have multiple tests for and while to collects the tests individually, it copies the raw out for each test which creates a fairly large json to be created when responding. not sure if this is something that you want or if you want each individual test to only contain the example output that I have as a json and includes the command to run the test: |
|
@DAC098 I think a lengthy raw out might be unavoidable and kind of serves the purpose in the interface where Output has the really detailed information (in cases where more info might be necessary) while the Outcome has a more concise result. In the outcome, either When custom assertion messages are used, are they displaying (in full) in the outcome? If so, that'd be perfect because that'd give us the power to choose custom messages that make the most sense for that particular exercise / test case! |
Closes #23
Summary
Testing