Skip to content

Modify PyTA report messages#1356

Merged
david-yz-liu merged 20 commits into
pyta-uoft:masterfrom
rachelzUT:modify-report-messages
Jun 19, 2026
Merged

Modify PyTA report messages#1356
david-yz-liu merged 20 commits into
pyta-uoft:masterfrom
rachelzUT:modify-report-messages

Conversation

@rachelzUT

@rachelzUT rachelzUT commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

Edited the PythonTA reports (plain text and HTML) to not show sections with no errors and only show 'good job' message once when there are no errors in either section. This eliminates repeated messages, allowing for a simpler and clearer report. Additionally edited the 'good job' message to include a random emoji.

...

Screenshots of your changes (if applicable)

Type of Change

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change that adds functionality) X
🐛 Bug fix (non-breaking change that fixes an issue)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

Before opening your pull request:

  • I have performed a self-review of my changes.
    • Check that all changed files included in this pull request are intentional changes.
    • Check that all changes are relevant to the purpose of this pull request, as described above.
  • I have added tests for my changes, if applicable.
    • This is required for all bug fixes and new features.
  • I have updated the project documentation, if applicable.
    • This is required for new features.
  • I have updated the project Changelog (this is required for all changes).
  • If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported by Coveralls.
  • I have requested a review from a project maintainer.

Questions and Comments

column-gap was added to .main-grid in stylesheet.css for aesthetic purposes, otherwise the title/table of contents and errors sections are spaced together quite closely.

Example of PyTA HTML report with no errors:
image

Example of PyTA plaintext report with no errors:
image

@coveralls

coveralls commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 27581017533

Coverage increased (+0.002%) to 90.931%

Details

  • Coverage increased (+0.002%) from the base build.
  • Patch coverage: 13 of 13 lines across 2 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 4058
Covered Lines: 3690
Line Coverage: 90.93%
Coverage Strength: 17.67 hits per line

💛 - Coveralls

@rachelzUT rachelzUT requested a review from david-yz-liu June 13, 2026 21:18

@david-yz-liu david-yz-liu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rachelzUT in addition to the inline comments I left, please also include a screenshot of the updated plaintext report

flex: 1;
grid-template-columns: min-content 1fr;
grid-template-rows: min-content 1fr;
column-gap: 3rem;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep properties in alphabetical order

}

.no-errors {
text-align: center;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep properties in alphabetical order

_PRE_LINE_NUM_SPACES = 0

no_err_message = "No problems detected, good job!"
NO_ERR_EMOJIS = ["🎉", "🥳", "🌟", "👍", "👏", "😊", "🎊", "🙌", "🕺"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is shared across the multiple reporters, define this and the no_err_message method in the base PythonTaReporter class

result += style_msgs_result

if not (code_msgs_result or style_msgs_result):
result += self.no_err_message()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to also display this message if level == "error" and there are no code messages, even when there are style messages. (This is also true of the HTML Reporter, so please double-check that.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change to plain_reporter.py. For the HTML reporter, the level argument doesn't appear to actually be utilized or affect the output, since the report is produced by display_messages which doesn't even accept level as an argument.

def print_messages(self, level="all"):
"""Do nothing to print messages, since all are displayed in a single HTML file."""
def display_messages(self, layout: BaseLayout) -> None:
"""Hook for displaying the messages of the reporter

@rachelzUT rachelzUT requested a review from david-yz-liu June 15, 2026 22:46

@david-yz-liu david-yz-liu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, @rachelzUT!

@david-yz-liu david-yz-liu merged commit 91780f2 into pyta-uoft:master Jun 19, 2026
30 checks passed
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.

3 participants