User experience for showing failed assertions

[denar90]

Following up with #97 and moving discussion here before creating one more PR :)

Some of the users are running into an issues when failed assertion results for resource-summary are not transparent when comparing with budget (for example - treosh/lighthouse-ci-action#13, part of the problem is fixed via #97).

E.g. I setup budget for 1KB, but actual value is ~17387 bytes.
What will be seen in console is something like this:

expected: <=100
found: 17387

For better UX, I propose something like we have in LH:

expected: <=1.00 KB
found: 16.98 KB

Here is comparison of logs before and after the proposal:

We actually need to define:

• are we ok with KB?
• what fields has to be transformed: resource-summary, others?

[patrickhulce]

We have this problem in the diff UI too when we need to know what the unit is for a particular numeric value, I think we need to develop a shared library in utils that, given an audit result and subproperty, can return the associated unit. We can move the logic that already exists in the diff UI to a shared util and use that in the CLI as well.

[exterkamp]

+1 this is a problem in LH core as well. It seems solved, but it actually isn't; it's most notable when looking at it in the context of i18n. Note the subtle differences in ru:

I'd love for all of this to be extracted to a common util that any consumer could use 😄

[brendankenny]

@exterkamp I think that's slightly different because all the units are available for those numbers (we just don't use them responsibly :). I added a note in GoogleChrome/lighthouse#7238 (comment) about solving that using the new Intl.NumberFormat unit formatting that came with Node 12.13 (the new LTS).

[patrickhulce]

Yeah agreed that's a slightly different problem with a different solution. Here we're talking about numericValue property on the audit result LHR and understanding what unit it's supposed to be. It isn't documented anywhere, it changes based on the audit, and it doesn't always match the unit/number that is in the displayValue.

[exterkamp]

@exterkamp I think that's slightly different because all the units are available for those numbers (we just don't use them responsibly :)

I was thinking about it in the context of "we know the context there, but we use it in different ways and bork it up and so standardizing around a different top level prop/library would be good" i.e. with Intl.NumberFormat we still need the metadata to say if it is in kilobytes to format it, and that data should be in the lhr to give the display value context. Solving that problem makes solving this lhci problem easier too.

Should the display value be baked into the report in a finalized form? ("displayValue": "10 кБ") and/or should it be represented in parts for processing at the viewer level? ("numericValue": 10, "numericUnit": "kilobytes")

I think including numericUnit would be good for this use case and for Lighthouse in general. I guess this was a roundabout way of me getting here...

[patrickhulce]

Gothcha, yeah definitely agree that numericUnit is the way to go in core 👍

[brendankenny]

numericUnit makes sense to pair with numericValue to help programmatic consumers (like lhci), but I don't see anywhere in that screenshot that'll be helped by it (we don't use numericValue anywhere in the renderer, for that matter, it's just for other LHR consumers).

displayValues are, for the most part, already correctly localized (e.g. the 0,0 сек in that screenshot). It's the functions like Util.formatSeconds that need updating to use the 'unit' style formatting that are causing the seen issues. And all those functions already do have access to the units for those values (in table columns, audit-details, or things with agreed-upon units like details.overallSavingsMs on opportunities), they just aren't localizing the unit part.

[patrickhulce]

In 6.0 we finally have numericUnit so we can address this.