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

feat: change test outputs to be html and add toggle buttons #630

Merged
merged 7 commits into from
Feb 27, 2024

Conversation

civsiv
Copy link
Contributor

@civsiv civsiv commented Feb 20, 2024

Summary page:
Screenshot 2024-02-20 at 11 06 40 (2)

Test output page (on load, all but first error collapsed) :
Screenshot 2024-02-20 at 11 07 02

Open all:
Screenshot 2024-02-20 at 11 07 14

Collapse all:
Screenshot 2024-02-20 at 11 07 23

@@ -1,5 +1,20 @@
<head>
<style>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some basic CSS to make headers more obviously clickable

Copy link
Contributor

@lukehesluke lukehesluke left a comment

Choose a reason for hiding this comment

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

@civsiv apologies if I'm using it wrong but this is what I've found, just by looking at the output HTML files:

Individual Test Page

  • ✅ On page load, all but the first error are collapsed

  • ✅ If I press "Toggle All Sections" the first time, all sections become un-collapsed. Then later presses toggle between all collapsed and all un-collapsed

  • ✅ If all sections are un-collapsed, the "Collapse All But First Error" button does as expected.

  • (Edit: Now fixed ✅) ❌ If all sections are already collapsed, the "Collapse All But First Error" button doesn't do anything

  • ✅ It's possible to collapse a section by clicking on the section header, (which also has all the visual markers of being clickable)

  • (Edit: Now fixed ✅) ❌ However, clicking on an already-collapsed section does not un-collapse it

  • (Edit: Now fixed ✅)< Return to Summary link (in the top left) is currently broken - it links to summary.md

  • Suggestion: Split the "Toggle All Sections" button into "Collapse all Sections" and "Show all Sections". Or maybe have some other way of indicating whether pressing the button will collapse or show? This is a random example but one I could think of which is: https://google.github.io/styleguide/lispguide.xml. They have the same button, but it uses an arrow to indicate whether it's showing or collapsing

  • Minor Suggestion: Rename the "Collapse All But First Error" button into "Show Only First Error"

  • Suggestion: Is there some tool/library that is easy to set up which can be used to make those code blocks a little more readable? Like syntax highlighting and maybe putting a box around them making it easy to copy all the code. Maybe hightlight.js?

Summary page

  • ❌ Make it possible to show/hide successful tests (maybe toggle button(s))?
  • ✅ Everything else looks good

@nickevansuk
Copy link
Collaborator

nickevansuk commented Feb 20, 2024

Further to Luke's comments:

  • CSS of H2s could be updated to include cursor: pointer;, to make it clear they're clickable (if they're not already)
  • Include a summary tick or cross at the start of the H2 line itself - to make it clear where the errors are when everything is collapsed.

The second point above is a little gnarly to get your head around given the structure of the logger (where the suite name is basically used to "look up" or "filter" things), but should be quite straightforward, so here's some thoughts of how to achieve this:

  • In the template, similar to the way ## {{{ renderSuiteName . }}} and {{#logsFor . "information"}} have the suite name passed in by ., include {{ validationIcon (statusFor .) }} (new function statusFor as a subexpression) just before it, inside {{#each activeSuites }} (so ## {{ validationIcon (statusFor .) }} {{{ renderSuiteName . }}}) (ref)
  • Create a new helper similar to logsFor called statusFor (ref)
  • Implement a statusFor similar to logsFor in the logger (ref). Use logic similar to get overallStatus() (ref), but additionally filtering on the suite name. Ideally abstract the logic out so it can be used in both places. Note the inheritance structure here - ReporterLogger extends BaseLogger - logsFor is in ReporterLogger, overallStatus is in BaseLogger.

@civsiv
Copy link
Contributor Author

civsiv commented Feb 22, 2024

New Zip:
output.zip

@nickevansuk
Copy link
Collaborator

Suggest adding a thin border to the top of the H2:

h2 {
    cursor: pointer;
    border-top-color: #CCC;
    border-top-width: 1px;
    border-top-style: solid;
    padding-top: 5px;
}

Copy link
Collaborator

@nickevansuk nickevansuk left a comment

Choose a reason for hiding this comment

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

This looks great. One thing that hasn't been tested here are the auth tests, that include embedded screenshots.

It's worth checking that these screenshots still render.

Assuming they do, then this is approved.

@civsiv
Copy link
Contributor Author

civsiv commented Feb 27, 2024

Screenshot 2024-02-27 at 10 13 11 Screenshots are unaffected!

@civsiv civsiv merged commit 908e50c into master Feb 27, 2024
10 checks passed
@civsiv civsiv deleted the feature/markdown-to-html-for-results branch February 27, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants