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

tests: update many test traces, support .json.gz #16007

Merged
merged 10 commits into from
May 30, 2024

Conversation

connorjclark
Copy link
Collaborator

Updates many old test traces. Also adds .json.gz support to asset-saver.js. I wanted to add fixtures for cnn.com, but they were far too big to commit without compression.

ref #15841

@connorjclark connorjclark requested a review from a team as a code owner May 21, 2024 00:34
@connorjclark connorjclark requested review from adamraine and removed request for a team May 21, 2024 00:34
Comment on lines 33 to 35
"optimistic": 2538,
"pessimistic": 2820,
"timing": 2679,
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use a more controlled trace to replace this? I don't envy the next person making changes to the simulator having to manually determine if any changes to these numbers are reasonable from a massive cnn trace.

Alternatively, if it needs to be more complex than what's in our smoke tests, in #14525 we had discussed using the test sites in https://github.com/ChromeDevTools/performance-stories as the source of generating test artifacts. Controlled, intentional, but also a decent variety of scenarios.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see any story there that has much JS work going on.

I can switch to something else. intercom-widget has a value here. But I don't know why for observed I'm getting this:

  1) Metrics: TotalBlockingTime
       should compute an observed value:
     LighthouseError: NO_TTI_CPU_IDLE_PERIOD
      at Interactive.findOverlappingQuietPeriods (file:///Users/cjamcl/src/lighthouse/core/computed/metrics/interactive.js:134:11)
      at Interactive.computeObservedMetric (file:///Users/cjamcl/src/lighthouse/core/computed/metrics/interactive.js:164:41)
      at Interactive.compute_ (file:///Users/cjamcl/src/lighthouse/core/computed/metrics/metric.js:91:21)

I wonder if user flows are ending too early?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@paulirish and I investigated the NO_TTI error and found an unrelated issue

@connorjclark connorjclark merged commit b2fd47f into main May 30, 2024
27 checks passed
@connorjclark connorjclark deleted the update-many-test-traces branch May 30, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants