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

[infra] Fix flaky ts-starter-kit tests #3157

Merged
merged 5 commits into from Jul 21, 2022
Merged

[infra] Fix flaky ts-starter-kit tests #3157

merged 5 commits into from Jul 21, 2022

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Jul 21, 2022

I think the flakes we have been seeing from the starter kit test were just timeouts. Since things are running in parallel, it sometimes takes longer for browser tests to start. The starter kits now run separately, after the other tests, so that they have less resource contention. We could also bump the browserStartTimeout, but since it's a user template repo, that seems weird.

Also:

@changeset-bot
Copy link

changeset-bot bot commented Jul 21, 2022

🦋 Changeset detected

Latest commit: a56cdc4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@lit-labs/context Patch
@lit-labs/motion Patch
@lit-labs/observers Patch
@lit-labs/react Patch
@lit-labs/scoped-registry-mixin Patch
@lit-labs/task Patch
lit-element Patch
lit-html Patch
lit Patch
@lit/reactive-element Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2022

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -4% - +5% (-1.58ms - +1.87ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 119.96ms - 125.94ms
  • lit-html-kitchen-sink: unsure 🔍 -3% - +5% (-1.49ms - +2.54ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -6% - +3% (-0.79ms - +0.41ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -5% - +1% (-4.16ms - +1.01ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -2% - +5% (-1.69ms - +3.99ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 1190.73ms - 1214.55ms
  • lit-html-kitchen-sink: unsure 🔍 -6% - +3% (-7.82ms - +3.85ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -1% - +3% (-4.25ms - +9.21ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -2% - +3% (-4.12ms - +4.45ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -1% - +2% (-6.73ms - +29.77ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 1220.13ms - 1244.91ms
  • reactive-element-list: unsure 🔍 -2% - +0% (-26.99ms - +3.80ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
119.96ms - 125.94ms-

update

VersionAvg timevs
1190.73ms - 1214.55ms-

update-reflect

VersionAvg timevs
1220.13ms - 1244.91ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
48.75ms - 51.25ms-unsure 🔍
-3% - +5%
-1.49ms - +2.54ms
unsure 🔍
-7% - +1%
-3.64ms - +0.78ms
tip-of-tree
tip-of-tree
47.89ms - 51.06msunsure 🔍
-5% - +3%
-2.54ms - +1.49ms
-unsure 🔍
-8% - +1%
-4.37ms - +0.45ms
previous-release
previous-release
49.61ms - 53.25msunsure 🔍
-2% - +7%
-0.78ms - +3.64ms
unsure 🔍
-1% - +9%
-0.45ms - +4.37ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
133.59ms - 139.76ms-unsure 🔍
-6% - +3%
-7.82ms - +3.85ms
unsure 🔍
-6% - +2%
-8.36ms - +2.35ms
tip-of-tree
tip-of-tree
133.71ms - 143.61msunsure 🔍
-3% - +6%
-3.85ms - +7.82ms
-unsure 🔍
-5% - +4%
-7.63ms - +5.59ms
previous-release
previous-release
135.30ms - 144.06msunsure 🔍
-2% - +6%
-2.35ms - +8.36ms
unsure 🔍
-4% - +6%
-5.59ms - +7.63ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
38.05ms - 41.00ms-unsure 🔍
-4% - +5%
-1.58ms - +1.87ms
unsure 🔍
-7% - +3%
-2.68ms - +1.19ms
tip-of-tree
tip-of-tree
38.50ms - 40.27msunsure 🔍
-5% - +4%
-1.87ms - +1.58ms
-unsure 🔍
-6% - +2%
-2.42ms - +0.65ms
previous-release
previous-release
39.02ms - 41.52msunsure 🔍
-3% - +7%
-1.19ms - +2.68ms
unsure 🔍
-2% - +6%
-0.65ms - +2.42ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
13.48ms - 14.18ms-unsure 🔍
-6% - +3%
-0.79ms - +0.41ms
unsure 🔍
-8% - +3%
-1.14ms - +0.38ms
tip-of-tree
tip-of-tree
13.53ms - 14.51msunsure 🔍
-3% - +6%
-0.41ms - +0.79ms
-unsure 🔍
-7% - +4%
-1.03ms - +0.64ms
previous-release
previous-release
13.54ms - 14.89msunsure 🔍
-3% - +8%
-0.38ms - +1.14ms
unsure 🔍
-5% - +7%
-0.64ms - +1.03ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
365.20ms - 375.59ms-unsure 🔍
-1% - +3%
-4.25ms - +9.21ms
unsure 🔍
-2% - +2%
-6.63ms - +8.04ms
tip-of-tree
tip-of-tree
363.63ms - 372.19msunsure 🔍
-2% - +1%
-9.21ms - +4.25ms
-unsure 🔍
-2% - +1%
-8.50ms - +4.94ms
previous-release
previous-release
364.51ms - 374.87msunsure 🔍
-2% - +2%
-8.04ms - +6.63ms
unsure 🔍
-1% - +2%
-4.94ms - +8.50ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
75.88ms - 77.97ms-unsure 🔍
-5% - +1%
-4.16ms - +1.01ms
unsure 🔍
-3% - +2%
-2.22ms - +1.61ms
tip-of-tree
tip-of-tree
76.13ms - 80.86msunsure 🔍
-1% - +5%
-1.01ms - +4.16ms
-unsure 🔍
-2% - +5%
-1.59ms - +4.12ms
previous-release
previous-release
75.63ms - 78.84msunsure 🔍
-2% - +3%
-1.61ms - +2.22ms
unsure 🔍
-5% - +2%
-4.12ms - +1.59ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
171.49ms - 177.03ms-unsure 🔍
-2% - +3%
-4.12ms - +4.45ms
unsure 🔍
-2% - +4%
-3.41ms - +6.10ms
tip-of-tree
tip-of-tree
170.82ms - 177.37msunsure 🔍
-3% - +2%
-4.45ms - +4.12ms
-unsure 🔍
-2% - +4%
-3.89ms - +6.25ms
previous-release
previous-release
169.04ms - 176.79msunsure 🔍
-3% - +2%
-6.10ms - +3.41ms
unsure 🔍
-4% - +2%
-6.25ms - +3.89ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
86.48ms - 90.50ms-unsure 🔍
-2% - +5%
-1.69ms - +3.99ms
unsure 🔍
-2% - +4%
-1.50ms - +3.35ms
tip-of-tree
tip-of-tree
85.34ms - 89.35msunsure 🔍
-4% - +2%
-3.99ms - +1.69ms
-unsure 🔍
-3% - +3%
-2.64ms - +2.19ms
previous-release
previous-release
86.22ms - 88.91msunsure 🔍
-4% - +2%
-3.35ms - +1.50ms
unsure 🔍
-3% - +3%
-2.19ms - +2.64ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1249.89ms - 1278.16ms-unsure 🔍
-1% - +2%
-6.73ms - +29.77ms
unsure 🔍
-1% - +2%
-17.88ms - +22.74ms
tip-of-tree
tip-of-tree
1240.96ms - 1264.04msunsure 🔍
-2% - +1%
-29.77ms - +6.73ms
-unsure 🔍
-2% - +1%
-27.69ms - +9.51ms
previous-release
previous-release
1247.01ms - 1276.18msunsure 🔍
-2% - +1%
-22.74ms - +17.88ms
unsure 🔍
-1% - +2%
-9.51ms - +27.69ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1305.96ms - 1325.54ms-unsure 🔍
-2% - +0%
-26.99ms - +3.80ms
faster ✔
0% - 2%
0.77ms - 32.39ms
tip-of-tree
tip-of-tree
1315.47ms - 1339.23msunsure 🔍
-0% - +2%
-3.80ms - +26.99ms
-unsure 🔍
-2% - +1%
-22.17ms - +12.21ms
previous-release
previous-release
1319.91ms - 1344.74msslower ❌
0% - 2%
0.77ms - 32.39ms
unsure 🔍
-1% - +2%
-12.21ms - +22.17ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

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

You legend!

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

🎉

@augustjk
Copy link
Member

While I am totally on board with this to make our pipeline smoother and have excitedly approved, a tiny reservation about this is that the starter kits are also meant as templates for users to potentially replicate for their own projects. Setting a config purely based on our workflow seems less ideal.

Though a longer timeout setting is pretty innocuous, and we're also considering revamping these hopefully soon so I don't think we need to worry so much about it.

Alternative way to deflake would be to run the starter kit tests independently, either in serial or a separate job, with the similar idea as we have separate build steps for those already in our monorepo.

@aomarks
Copy link
Member Author

aomarks commented Jul 21, 2022

While I am totally on board with this to make our pipeline smoother and have excitedly approved, a tiny reservation about this is that the starter kits are also meant as templates for users to potentially replicate for their own projects. Setting a config purely based on our workflow seems less ideal.

Though a longer timeout setting is pretty innocuous, and we're also considering revamping these hopefully soon so I don't think we need to worry so much about it.

Alternative way to deflake would be to run the starter kit tests independently, either in serial or a separate job, with the similar idea as we have separate build steps for those already in our monorepo.

Good point. Done.

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

Thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants