lighthouse-ci icon indicating copy to clipboard operation
lighthouse-ci copied to clipboard

CLS diff not showing on compare screen

Open kudja opened this issue 4 years ago • 10 comments

Describe the bug I have 2 builds with different CLS Build 1 - CLS = 0.001 Build 2 - CLS = 0.026 When I compare this 2 builds it will not show CLS in metrics (this works with any threshold)

To Reproduce Steps to reproduce the behavior:

  1. Prepare 2 builds with different CLS
  2. Compare them

Expected behavior See CLS diff same way as any other metrics.

Logs/Screenshots I think it's clear w/o any screenshots

Environment (please complete the following information):

  • Server - latest docker server
  • Client - latest docker client

kudja avatar Sep 13 '21 09:09 kudja

Thanks for filing @Kudja! I'm not able to reproduce this with just the steps provided though.

image

(Note that the diff is small relative to CLS thresholds so it is represented by "0.0" but it does show up)

Anything additional about your environment or those builds you can share?

patrickhulce avatar Sep 13 '21 14:09 patrickhulce

Hi @patrickhulce, thanks for your response.

I'm using the official docker-client to gather stats, and docker server to show them. I just installed them a few days ago, so I believe they have the latest versions. Not sure what else can affect these values. In config, I'm using perf preset for lighthouse and I have such result in compare image CLS is now in Unchanged Audits. And if I'll open reports for them, then I can see the difference in CLS

If this can help somehow - here are compare link - https://lhci.d1.mageassist.com/app/projects/prilla.kudja.d1.mageassist.com/compare/9a0cac39e56d?baseBuild=14caf986-329f-4b30-af6e-b41b7288a5e6&baseUrl=https%3A%2F%2Fprilla.kudja.d1.mageassist.com%2Fus%2Fnicotine-pouches&compareUrl=https%3A%2F%2Fprilla.kudja.d1.mageassist.com%2Fus%2Fnicotine-pouches

kudja avatar Sep 13 '21 19:09 kudja

It looks like some rounding issue. When you have very small value of some metric on any side (Base / Compare) it looks like it rounding it to zero and in that case it will always show no diff.

I tested few different builds and If I have CLS = 0 - it's no matter which value we have on the compare page - it will always show no diff. If I have CLS = 0.001 then in some cases it will show the diff, in some not If I have CLS with some bigger values, it will always show diff

kudja avatar Sep 14 '21 09:09 kudja

Hi @patrickhulce

I found what is causing the issue. We have different CLS numericValue, but score=1 for both. And in that case, this check https://github.com/GoogleChrome/lighthouse-ci/blob/81a18a3378b0e1307c9b84e0360b7c4f00a6ee7b/packages/utils/src/audit-diff-finder.js#L668-L676 will pass and return it as no diff. But It doesn't look like a good idea to hide such changes, maybe better to add some config param or checkbox to show/hide results we are considering as flaky?

kudja avatar Sep 20 '21 15:09 kudja

It doesn't look like a good idea to hide such changes

Not for performance metrics, I agree. We'll add an exception here for performance metrics 👍

Good first issue if anyone would like to contribute :)

patrickhulce avatar Sep 20 '21 15:09 patrickhulce

Hi @patrickhulce I would like to work on this. Any insights upon implementation and getting started?

sohamsshah avatar Oct 21 '21 06:10 sohamsshah

great @sohamsshah! in the section that https://github.com/GoogleChrome/lighthouse-ci/issues/689#issuecomment-923016077 highlighted, make an exception for any audits that are metrics (either a hard-coded list of IDs or checking membership in a group called metrics works for me)

patrickhulce avatar Oct 21 '21 12:10 patrickhulce

either a hard-coded list of IDs or checking membership in a group called metrics works for me

I'm not very good in LH roadmap/updates, but may be that's not a best idea to use hardcoded values, as on any lighthouse report update we can get broken functionality. May be better to pass here to function categories related to this audit or get categories list from some global scope and do compare.

What do you think about this?

kudja avatar Oct 21 '21 20:10 kudja

@Kudja I guess if we compare the group of the audit to be metrics, it won't be hard coded because I think it very least probable to have change of group of an audit with any lighthouse future release and in fact the same check has been already used multiple times in the existing code. However, @patrickhulce having said that, if we exclude all the metrics audits from here, wouldn't it affect FCP, LCP, FID and other metrics?

vikasrohit avatar Apr 15 '22 06:04 vikasrohit

@patrickhulce create PR https://github.com/GoogleChrome/lighthouse-ci/pull/784 for this issue, please let me know the feedback.

vikasrohit avatar Apr 18 '22 03:04 vikasrohit