Fix frontend test errors

Background: Upon running the frontend tests, you can see there are many unexpected logs being printed out. This ranges from: 1. 404 Network Requests ([Example](https://github.com/GoogleChrome/webstatus.dev/actions/runs/12553689104/job/35001422582#step:6:10143)) 2. Uncaught exceptions ([Example](https://github.com/GoogleChrome/webstatus.dev/actions/runs/12553689104/job/35001422582#step:6:10178)) 3. Inefficient rendering ([Example](https://github.com/GoogleChrome/webstatus.dev/actions/runs/12553689104/job/35001422582#step:6:10338)) 4. An error was thrown in a Promise outside a test ([Example](https://github.com/GoogleChrome/webstatus.dev/actions/runs/12553689104/job/35001422582#step:6:10173)) **Case 1** Problem: - Tests are missing static resources Remedy: - Copy the static resources to same folder as the compiled tests. - Shoelace-only assets: Adjust the test html in web-test-runner.config.mjs to set the path for shoelace assets like we do in our main [index.ts](https://github.com/GoogleChrome/webstatus.dev/blob/4b8b4696cd20c37bec10b187190863266593bd4d/frontend/src/static/js/index.ts#L47) **Case 2** Remedy: Adjust the individual tests to fix the problems **Case 3** Remedy: - Remove unnecessary reactivity from certain class members for each instance - Specifically for `sl-tree-item`: Add lazy attribute. Its reactivity was causing internal re-rendering. After reading the shoelace code, I saw that setting lazy could help.
GoogleChrome · Jan 2, 2025 · 324eea2 · 324eea2
1 parent df141c5
commit 324eea2
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 51 deletions.
diff --git a/frontend/package.json b/frontend/package.json
@@ -8,16 +8,16 @@
   "main": "index.js",
   "type": "module",
   "scripts": {
-    "build": "npm run tsc-clean && tsc && npx rollup -c",
-    "clean": "rm -rf build && rm -rf static && rm -rf dist && rm -rf node_modules && rm -rf .postinstall",
+    "build": "npm run clean && tsc && npm run cp-imgs && npx rollup -c",
+    "clean": "rm -rf build && rm -rf static && rm -rf dist && tsc --build --clean",
     "lint": "cd .. && prettier frontend/ --check && cd frontend && eslint \"**/*.{js,ts}\" && lit-analyzer \"src/**/*.{js,ts}\" && tsc --noEmit",
     "lint-fix": "cd .. && prettier frontend/ --write && cd frontend && eslint \"**/*.{js,ts}\" --fix",
     "local": "npm run build && TODO",
     "postinstall": "./scripts/postinstall.js",
     "start": "node dist/server/index.js",
-    "test": "npm run tsc-clean && tsc && wtr --coverage --node-resolve --playwright --browsers chromium firefox webkit",
-    "tsc-clean": "tsc --build --clean",
-    "test:watch": "tsc && concurrently -k --raw \"tsc --watch --preserveWatchOutput\"  \"wtr --watch --coverage --node-resolve --playwright --browsers chromium firefox webkit\" "
+    "cp-imgs": "mkdir -p build/public/img && cp -v -r src/static/img/* build/public/img && cp -v .postinstall/static/img/* build/public/img",
+    "test": "npm run build && npm run cp-imgs && wtr --coverage --node-resolve --playwright --browsers chromium firefox webkit",
+    "test:watch": "npm run build && concurrently -k --raw \"tsc --watch --preserveWatchOutput\"  \"wtr --watch --coverage --node-resolve --playwright --browsers chromium firefox webkit\" "
   },
   "devDependencies": {
     "@browser-logos/chrome": "^2.0.0",

diff --git a/frontend/rollup.config.mjs b/frontend/rollup.config.mjs
@@ -43,12 +43,8 @@ export default [
       }),
       copy({
         targets: [
-          // Copy all files in img recursively.
-          // Currently copying svg files from https://github.com/mdn/yari/tree/main/client/src/assets/icons/baseline
-          {src: 'src/static/img/**', dest: 'dist/static/public/img'},
           // Copy the img files
-          // Currently copying img files from ./scripts/postinstall.js
-          {src: '.postinstall/static/img/*', dest: 'dist/static/public/img'},
+          {src: 'build/public/img/*', dest: 'dist/static/public/img'},
           // Copy the html file
           {src: 'src/static/index.html', dest: 'dist/static'},
         ],

diff --git a/frontend/src/static/js/components/test/webstatus-feature-page.test.ts b/frontend/src/static/js/components/test/webstatus-feature-page.test.ts
@@ -24,8 +24,8 @@ import {render} from 'lit';
 describe('webstatus-feature-page', () => {
   let el: FeaturePage;
   let renderDescriptionSpy: sinon.SinonSpy;
+  const location = {params: {featureId: 'some-feature'}, search: ''};
   beforeEach(async () => {
-    const location = {params: {featureId: 'some-feature'}, search: ''};
     el = await fixture<FeaturePage>(
       html`<webstatus-feature-page
         .location=${location}
@@ -144,7 +144,9 @@ describe('webstatus-feature-page', () => {
 
     beforeEach(async () => {
       element = await fixture(
-        html`<webstatus-feature-page></webstatus-feature-page>`,
+        html`<webstatus-feature-page
+          .location=${location}
+        ></webstatus-feature-page>`,
       );
       hostElement = document.createElement('div');
 
@@ -244,7 +246,9 @@ describe('webstatus-feature-page', () => {
 
     beforeEach(async () => {
       element = await fixture(
-        html`<webstatus-feature-page></webstatus-feature-page>`,
+        html`<webstatus-feature-page
+          .location=${location}
+        ></webstatus-feature-page>`,
       );
       element.endDate = new Date('2024-01-01');
       hostElement = document.createElement('div');

diff --git a/frontend/src/static/js/components/test/webstatus-gchart.test.ts b/frontend/src/static/js/components/test/webstatus-gchart.test.ts
@@ -14,54 +14,81 @@
  * limitations under the License.
  */
 
-import {assert, fixture, html} from '@open-wc/testing';
+import {assert, html} from '@open-wc/testing';
 import '../../services/webstatus-gcharts-loader-service.js';
 import '../webstatus-gchart.js';
 import {type WebstatusGChart} from '../webstatus-gchart.js';
 import {render} from 'lit';
 import {type WebstatusGChartsLoaderService} from '../../services/webstatus-gcharts-loader-service.js';
+import sinon from 'sinon';
 
 describe('webstatus-gchart', () => {
-  it('can receive loaded state via loader context', async () => {
-    const component = await fixture<WebstatusGChartsLoaderService>(
-      html`<webstatus-gcharts-loader-service>
-        <webstatus-gchart></webstatus-gchart>
-      </webstatus-gcharts-loader-service>`,
-    );
+  let mockResizeObserver: sinon.SinonStub;
+  let component: WebstatusGChart;
+  let loaderComponent: WebstatusGChartsLoaderService;
 
-    assert.exists(component);
-    await component.updateComplete;
-    await component.waitForGChartsLibraryLoaded();
+  beforeEach(async () => {
+    // Mock the ResizeObserver constructor to prevent
+    // "ResizeObserver loop completed with undelivered notifications."
+    // errors that can happen intermittently in tests.
+    // An added benefit is that we can control the resize.
+    mockResizeObserver = sinon.stub(window, 'ResizeObserver').callsFake(() => ({
+      observe: sinon.stub(),
+      disconnect: sinon.stub(),
+    }));
 
-    const childComponent = component.querySelector(
-      'webstatus-gchart',
-    ) as WebstatusGChart;
-    assert.exists(childComponent);
-    await childComponent.updateComplete;
-    assert.equal(childComponent.gchartsLibraryLoaded, true);
-  });
-
-  it('can subscribe to the parent gchart loader', async () => {
-    // This also tests adding components via lit render.
+    // Create a root div and append it to the body
     const root = document.createElement('div');
     document.body.appendChild(root);
+
+    // Use render to create the components
     render(
       html`
         <webstatus-gcharts-loader-service>
-          <webstatus-gchart></webstatus-gchart>
+          <div id="test-container">
+            <webstatus-gchart
+              .containerId="${'test-container'}"
+            ></webstatus-gchart>
+          </div>
         </webstatus-gcharts-loader-service>
       `,
       root,
     );
-    const loader = root.querySelector(
+
+    // Get the components
+    loaderComponent = root.querySelector(
       'webstatus-gcharts-loader-service',
     ) as WebstatusGChartsLoaderService;
-    assert.exists(loader);
-    await loader.updateComplete;
-    await loader.waitForGChartsLibraryLoaded();
+    component = loaderComponent.querySelector(
+      'webstatus-gchart',
+    ) as WebstatusGChart;
+
+    await loaderComponent.updateComplete;
+    await loaderComponent.waitForGChartsLibraryLoaded();
+    await component.updateComplete;
+  });
+
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  it('can receive loaded state via loader context', async () => {
+    assert.equal(component.gchartsLibraryLoaded, true);
+  });
+
+  it('redraws the chart on resize', async () => {
+    // Spy on the chartWrapper.draw method (make sure chartWrapper is initialized)
+    const drawSpy = sinon.spy(component.chartWrapper!, 'draw');
+
+    // Simulate a resize
+    const resizeObserverCallback = mockResizeObserver.args[0][0];
+    resizeObserverCallback([
+      {
+        contentRect: {width: 200, height: 100},
+      },
+    ]);
 
-    const gchart = root.querySelector('webstatus-gchart') as WebstatusGChart;
-    await gchart.updateComplete;
-    assert.equal(gchart.gchartsLibraryLoaded, true);
+    // Assert that chartWrapper.draw was called
+    assert.isTrue(drawSpy.calledOnce);
   });
 });
diff --git a/frontend/src/static/js/components/webstatus-feature-page.ts b/frontend/src/static/js/components/webstatus-feature-page.ts
@@ -133,7 +133,6 @@ export class FeaturePage extends LitElement {
   @state()
   featureMetadata?: {can_i_use?: CanIUseData; description?: string} | undefined;
 
-  @state()
   featureId!: string;
 
   location!: {params: {featureId: string}; search: string}; // Set by router.

diff --git a/frontend/src/static/js/components/webstatus-overview-filters.ts b/frontend/src/static/js/components/webstatus-overview-filters.ts
@@ -151,7 +151,6 @@ export class WebstatusOverviewFilters extends LitElement {
   @state()
   exportDataStatus: TaskStatus = TaskStatus.INITIAL;
 
-  @state()
   // A function that returns an array of all features via apiClient.getAllFeatures
   allFeaturesFetcher:
     | undefined

diff --git a/frontend/src/static/js/components/webstatus-sidebar-menu.ts b/frontend/src/static/js/components/webstatus-sidebar-menu.ts
@@ -253,7 +253,7 @@ export class WebstatusSidebarMenu extends LitElement {
     const bookmarkIcon = isQueryActive ? 'bookmark-star' : 'bookmark';
 
     return html`
-      <sl-tree-item id=${bookmarkId} ?selected=${isQueryActive}>
+      <sl-tree-item id=${bookmarkId} ?selected=${isQueryActive} lazy>
         <a class="bookmark-link" href="${bookmarkUrl}">
           <sl-icon name="${bookmarkIcon}"></sl-icon> ${bookmark.name}
         </a>
@@ -269,7 +269,8 @@ export class WebstatusSidebarMenu extends LitElement {
 
         <sl-tree-item
           id="${NavigationItemKey.FEATURES}"
-          expanded=${this.isFeaturesDropdownExpanded}
+          .expanded=${this.isFeaturesDropdownExpanded}
+          lazy
         >
           <sl-icon name="menu-button"></sl-icon>
           <a
@@ -288,7 +289,7 @@ export class WebstatusSidebarMenu extends LitElement {
 
         <sl-divider aria-hidden="true"></sl-divider>
 
-        <sl-tree-item class="report-issue-item">
+        <sl-tree-item class="report-issue-item" lazy>
           <sl-icon name="github"></sl-icon>
           <a
             class="report-issue-link"
@@ -298,7 +299,7 @@ export class WebstatusSidebarMenu extends LitElement {
           >
         </sl-tree-item>
 
-        <sl-tree-item class="about-item">
+        <sl-tree-item class="about-item" lazy>
           <sl-icon name="info-circle"></sl-icon>
           <a class="about-link" href="${ABOUT_PAGE_LINK}" target="_blank"
             >About</a

diff --git a/frontend/src/static/js/services/webstatus-firebase-app-service.ts b/frontend/src/static/js/services/webstatus-firebase-app-service.ts
@@ -15,7 +15,7 @@
  */
 
 import {provide} from '@lit/context';
-import {customElement, property, state} from 'lit/decorators.js';
+import {customElement, property} from 'lit/decorators.js';
 import {
   FirebaseApp,
   firebaseAppContext,
@@ -34,7 +34,6 @@ export class WebstatusFirebaseAppService extends ServiceElement {
   settings?: FirebaseSettings;
 
   @provide({context: firebaseAppContext})
-  @state()
   firebaseApp?: FirebaseApp;
 
   protected firstUpdated(): void {

diff --git a/frontend/web-test-runner.config.mjs b/frontend/web-test-runner.config.mjs
@@ -24,13 +24,24 @@ export default /** @type {import("@web/test-runner").TestRunnerConfig} */ ({
   },
 
   // in a monorepo you need to set the root dir to resolve modules
-  rootDir: '../../',
+  rootDir: 'build/',
 
   files: [
     // Have to compile tests
     // Taken from https://github.com/open-wc/create/blob/master/src/generators/testing-wtr-ts/templates/static/web-test-runner.config.mjs
-    'build/**/test/*.test.js',
+    '**/test/*.test.js',
   ],
+  testRunnerHtml: testFramework => `
+  <html>
+    <body>
+      <script type="module" src="${testFramework}"></script>
+      <script type="module">
+        import { setBasePath } from '@shoelace-style/shoelace/dist/utilities/base-path.js';
+        setBasePath('/public/img/shoelace');
+      </script>
+    </body>
+  </html>
+  `,
 
   /** Filter out lit dev mode logs */
   filterBrowserLogs(log) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -133,7 +133,6 @@ export class FeaturePage extends LitElement { @@
       @state()
       featureMetadata?: {can_i_use?: CanIUseData; description?: string} | undefined;
-      @state()
       featureId!: string;
       location!: {params: {featureId: string}; search: string}; // Set by router.
@@ Expand Down @@