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] Replace custom test module remapping with package exports #3132

Merged
merged 19 commits into from Jul 15, 2022

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Jul 14, 2022

Switches from a custom web test runner plugin to the now built-in export conditions configuration, in order to switch between dev and prod builds in our browser tests.

Background

Some of our packages contain 2 separate builds: dev and prod. Dev is the raw output from TypeScript and includes extra debugging features. Prod is the minified build that users will get in production, and excludes those debugging features.

The two modes are described in our package.json files using export conditions. By default, users get the prod build. If they set the development export condition in tools like rollup, webpack, node, they will get the dev build.

We run all of our tests against both builds to get full coverage. Previously, we did this with a custom plugin for web test runner which rewrote development/ paths to production paths when needed. We did this because web test runner did not, at the time, have support for export conditions.

Now, web test runner does have support for export conditions.

Changes

  • Switches all tests to import their respective libraries using package self-references, instead of relative paths. This allows export conditions to take effect even from within the same package.

  • Switches to the NodeNext mode for the module and moduleResolution settings in our tsconfig.json files. This is needed for package self-references.

  • Adds types entries to the export conditions in our package.json files. This is needed when using NodeNext, and is how TypeScript discovers the typings for each specific package export. (Side note, in the future, this feature will allow us to remove the .d.ts files that we currently duplicate into the top-level of the package, but we can't do that until our users are also using NodeNext).

  • Added a new web test runner middleware that enforces that we only import dev sources in dev mode, and prod sources in prod mode. It serves a 403 forbidden when this is violated, which fails the test. This should prevent us from accidentally using relative imports in tests in the future.

  • Deleted the old path-rewriting web test runner plugin, which is no longer needed.

  • Upgraded all @web/test-runner and @web/dev-server packages, and consolidated them where possible into the top-level package.json for easier upgrades.

Fixes #2844
Fixes #3091

@changeset-bot
Copy link

changeset-bot bot commented Jul 14, 2022

🦋 Changeset detected

Latest commit: 69d999e

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

This PR includes changesets to release 9 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/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 14, 2022

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -1% - +4% (-0.20ms - +0.99ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 72.10ms - 72.71ms
  • lit-html-kitchen-sink: unsure 🔍 -0% - +3% (-0.13ms - +0.78ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -5% - +7% (-0.56ms - +0.72ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -0% - +2% (-0.05ms - +1.14ms)
    this-change vs tip-of-tree
  • reactive-element-list: slower ❌ 0% - 2% (0.05ms - 1.32ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 683.96ms - 687.94ms
  • lit-html-kitchen-sink: unsure 🔍 -3% - +7% (-2.08ms - +5.12ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -4% - +8% (-11.41ms - +22.38ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -0% - +2% (-0.23ms - +2.11ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -0% - +1% (-0.87ms - +5.64ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 699.10ms - 702.80ms
  • reactive-element-list: unsure 🔍 -0% - +2% (-2.43ms - +16.57ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
72.10ms - 72.71ms-

update

VersionAvg timevs
683.96ms - 687.94ms-

update-reflect

VersionAvg timevs
699.10ms - 702.80ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
27.94ms - 28.83ms-unsure 🔍
-0% - +3%
-0.13ms - +0.78ms
unsure 🔍
-2% - +3%
-0.60ms - +0.71ms
tip-of-tree
tip-of-tree
27.96ms - 28.15msunsure 🔍
-3% - +0%
-0.78ms - +0.13ms
-unsure 🔍
-3% - +1%
-0.77ms - +0.22ms
previous-release
previous-release
27.85ms - 28.81msunsure 🔍
-2% - +2%
-0.71ms - +0.60ms
unsure 🔍
-1% - +3%
-0.22ms - +0.77ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
76.22ms - 82.93ms-unsure 🔍
-3% - +7%
-2.08ms - +5.12ms
unsure 🔍
-4% - +6%
-3.01ms - +4.60ms
tip-of-tree
tip-of-tree
76.76ms - 79.36msunsure 🔍
-6% - +3%
-5.12ms - +2.08ms
-unsure 🔍
-4% - +2%
-2.93ms - +1.49ms
previous-release
previous-release
76.99ms - 80.57msunsure 🔍
-6% - +4%
-4.60ms - +3.01ms
unsure 🔍
-2% - +4%
-1.49ms - +2.93ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
25.66ms - 26.78ms-unsure 🔍
-1% - +4%
-0.20ms - +0.99ms
unsure 🔍
-1% - +4%
-0.23ms - +0.96ms
tip-of-tree
tip-of-tree
25.62ms - 26.02msunsure 🔍
-4% - +1%
-0.99ms - +0.20ms
-unsure 🔍
-1% - +1%
-0.32ms - +0.25ms
previous-release
previous-release
25.65ms - 26.06msunsure 🔍
-4% - +1%
-0.96ms - +0.23ms
unsure 🔍
-1% - +1%
-0.25ms - +0.32ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
10.44ms - 11.05ms-unsure 🔍
-5% - +7%
-0.56ms - +0.72ms
unsure 🔍
-1% - +6%
-0.12ms - +0.60ms
tip-of-tree
tip-of-tree
10.11ms - 11.23msunsure 🔍
-7% - +5%
-0.72ms - +0.56ms
-unsure 🔍
-4% - +7%
-0.43ms - +0.75ms
previous-release
previous-release
10.31ms - 10.70msunsure 🔍
-6% - +1%
-0.60ms - +0.12ms
unsure 🔍
-7% - +4%
-0.75ms - +0.43ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
271.64ms - 299.06ms-unsure 🔍
-4% - +8%
-11.41ms - +22.38ms
unsure 🔍
-8% - +6%
-22.39ms - +17.97ms
tip-of-tree
tip-of-tree
269.99ms - 289.73msunsure 🔍
-8% - +4%
-22.38ms - +11.41ms
-unsure 🔍
-9% - +3%
-25.49ms - +10.09ms
previous-release
previous-release
272.76ms - 302.37msunsure 🔍
-6% - +8%
-17.97ms - +22.39ms
unsure 🔍
-4% - +9%
-10.09ms - +25.49ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
51.71ms - 52.71ms-unsure 🔍
-0% - +2%
-0.05ms - +1.14ms
unsure 🔍
-1% - +2%
-0.39ms - +0.85ms
tip-of-tree
tip-of-tree
51.34ms - 52.00msunsure 🔍
-2% - +0%
-1.14ms - +0.05ms
-unsure 🔍
-2% - +0%
-0.80ms - +0.18ms
previous-release
previous-release
51.61ms - 52.34msunsure 🔍
-2% - +1%
-0.85ms - +0.39ms
unsure 🔍
-0% - +2%
-0.18ms - +0.80ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
108.20ms - 110.14ms-unsure 🔍
-0% - +2%
-0.23ms - +2.11ms
unsure 🔍
-1% - +2%
-0.63ms - +1.67ms
tip-of-tree
tip-of-tree
107.58ms - 108.88msunsure 🔍
-2% - +0%
-2.11ms - +0.23ms
-unsure 🔍
-1% - +0%
-1.32ms - +0.49ms
previous-release
previous-release
108.02ms - 109.27msunsure 🔍
-2% - +1%
-1.67ms - +0.63ms
unsure 🔍
-0% - +1%
-0.49ms - +1.32ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
54.32ms - 55.20ms-slower ❌
0% - 2%
0.05ms - 1.32ms
unsure 🔍
-1% - +1%
-0.59ms - +0.80ms
tip-of-tree
tip-of-tree
53.61ms - 54.54msfaster ✔
0% - 2%
0.05ms - 1.32ms
-unsure 🔍
-2% - +0%
-1.29ms - +0.14ms
previous-release
previous-release
54.11ms - 55.19msunsure 🔍
-1% - +1%
-0.80ms - +0.59ms
unsure 🔍
-0% - +2%
-0.14ms - +1.29ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
674.13ms - 679.53ms-unsure 🔍
-0% - +1%
-0.87ms - +5.64ms
unsure 🔍
-1% - +0%
-3.80ms - +2.72ms
tip-of-tree
tip-of-tree
672.64ms - 676.26msunsure 🔍
-1% - +0%
-5.64ms - +0.87ms
-faster ✔
0% - 1%
0.35ms - 5.50ms
previous-release
previous-release
675.54ms - 679.20msunsure 🔍
-0% - +1%
-2.72ms - +3.80ms
slower ❌
0% - 1%
0.35ms - 5.50ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
783.36ms - 794.03ms-unsure 🔍
-0% - +2%
-2.43ms - +16.57ms
unsure 🔍
-1% - +1%
-6.09ms - +11.62ms
tip-of-tree
tip-of-tree
773.77ms - 789.48msunsure 🔍
-2% - +0%
-16.57ms - +2.43ms
-unsure 🔍
-2% - +1%
-14.87ms - +6.26ms
previous-release
previous-release
778.87ms - 793.00msunsure 🔍
-1% - +1%
-11.62ms - +6.09ms
unsure 🔍
-1% - +2%
-6.26ms - +14.87ms
-

tachometer-reporter-action v2 for Benchmarks

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.

Awesome!

Copy link
Collaborator

@justinfagnani justinfagnani left a comment

Choose a reason for hiding this comment

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

Awesome work!

@aomarks aomarks merged commit 2fe2053 into main Jul 15, 2022
@aomarks aomarks deleted the dev-imports branch July 15, 2022 17:51
aomarks added a commit that referenced this pull request Jul 20, 2022
aomarks added a commit that referenced this pull request 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:

- Upgrades dependencies
- Removes some unnecessary devDeps from virtualizer
- Adds a changelog entry from #3132 that I forgot (unrelated, just throwing it in)
This was referenced Jul 21, 2022
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.

[infra] Something broken for testing in recent @web/dev-server [infra] Remove custom import remapping
3 participants