feat: make browsertest failures standout #4148

dhuang612 · 2020-01-09T19:59:21Z

Description of the Change

resolves #792 created a bar that starts out as green when the tests start.
Then if one of the tests fails bar will turn red to indicate test failure.
From the previous attempt learned that code coverage is decreased.
Will work on creating a test for this to get this resolved.

Alternate Designs

Why should this be in core?

Benefits

Allows for less eye movement to notice tests that don't pass when run from google chrome.

Possible Drawbacks

there shouldn't be any side effects because the changes are gui level

changes made are an enhancement.

when the tests first begin:

when a failure is detected:

coveralls · 2020-01-09T20:09:09Z

Coverage decreased (-0.03%) to 94.378% when pulling 35fd52e on dhuang612:make-browsertest-failures-standout into 9e0369b on mochajs:master.

dhuang612 · 2020-01-17T01:51:35Z

okay, so there aren't tests for the html reporter.

dhuang612 · 2020-01-20T18:53:39Z

reduced the size of the bar and gave them curved edges

success:

failure:

Please let me know if anything else needs to be changed

pedro-qwkin · 2020-08-09T18:02:30Z

Why isn't this merged?

setting package-lock.json back to last update added in sass and updated css class to use extend updated test files to expect sass

JoshuaKGoldberg

I think we'll want to have some more discussion into what this UI should look like. Proposal: can we have that in the backing issue, #792? That way we can figure out what it should look like a bit more before jumping into the implementation.

JoshuaKGoldberg · 2024-03-01T18:07:14Z

package.json

@@ -144,6 +144,7 @@
    "rollup-plugin-node-globals": "^1.4.0",
    "rollup-plugin-node-polyfills": "^0.2.1",
    "rollup-plugin-visualizer": "^4.1.0",
+    "sass": "^1.39.2",


[Structure] sass is a pretty big dependency to use for exactly one @extend. I don't think it's justified here. Let's revert the change from .css to .sass, unless there's a strong reason for it?

Removed in #5218 ;)

JoshuaKGoldberg · 2024-03-01T18:09:13Z

mocha.sass

+}
+
+#mocha-stats.fail {
+ @extend #mocha-stats;


[Style] I don't think this @extend is really needed. We can achieve the same result in raw CSS, such as with CSS variables or different class names.

Thinking in terms of semantic class names, I think it'd make sense to have a class like .fail, .pass, etc. that each define their own colors.

Answering #4148 (comment): no, CSS can overcome these issues. 🙂

JoshuaKGoldberg · 2024-03-01T18:24:44Z

lib/reporters/html.js

@@ -111,7 +111,6 @@ function HTML(runner, options) {
      hideSuitesWithout('test fail');
    }
  });
-


[Style] Nit: unnecessary change?

JoshuaKGoldberg · 2024-03-01T18:36:56Z

mocha.sass

Looking at the styles:

Pass Fail

A few things that make me not a fan yet:

The new colors are hard to read on top of, to the point of not having accessible color contrast.

The colors also don't match the existing aesthetic of the page. The page right now has very minimalist colors and a subtle design. This new green and red is pretty glaring. We should go with something that doesn't clash so much.

It's a little inconsistent in how its laid out: more padding on the top vs. the bottom, and less horizontal padding than other UI elements.

I'm not sure what would be the "right" approach. A few things come to mind, but I haven't thought very deeply:

Using much thinner border lines and/or much less saturated colors

Even switching to an nprogress-style bar on the stats header or the page

Instead of a background or border, using bold & colored text

(I'm not convinced any of that is better, just ideating)

JoshuaKGoldberg · 2024-07-02T16:49:01Z

👋 ping @dhuang612, do you still have time for this?

JoshuaKGoldberg · 2024-08-06T15:09:35Z

Closing out as it's been a while since PR activity. If anybody wants to take this over, please do - and post a co-author attribution if you take code from this PR. Cheers! 🤎

mark-wiemer · 2024-09-29T05:44:47Z

@JoshuaKGoldberg I'll be coming to this one as a software engineer at Microsoft specializing in a11y :) I'll be contributing as an individual, not on behalf of Microsoft. Please let me know if you have further comments before I start resolving the existing ones. I'll probably be asking for some help in the discord as well :) thanks again for picking up this project!

outsideris added the area: browser browser-specific label Jan 19, 2020

dhuang612 mentioned this pull request Jan 21, 2020

🚀 Feature: Highlight browser failures in red #792

Closed

outsideris added the area: reporters involving a specific reporter label Nov 10, 2020