zen-audio-player / zen-audio-player.github.io

Listen to YouTube videos, without the distracting visuals.
https://zen-audio-player.github.io/
MIT License
228 stars 180 forks source link

Replace bower packages with corresponding unpkg URLs #434

Closed voidCounter closed 2 months ago

voidCounter commented 2 months ago

Context

Closes #432

Description

This PR includes the following changes:

Checklist

Issues

I'm currently facing one or two test failures related to the timeout of the "Demo" and "Form" tests. Here's the error message for the "Demo" test:

1) Demo
       should play the demo when demo button is clicked:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/zen-audio-player.github.io/test/demo.js)
      at listOnTimeout (node:internal/timers:573:17)
      at process.processTimers (node:internal/timers:514:7)
vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zen-audio-player-github-io ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 6:04am
shakeelmohamed commented 2 months ago

@voidCounter Really great work here, and thank you for updating the tests as well!

The failing test is a CORS issue due to accessing remote files without running the site through a server.

The proper fix is starting a webserver in the test, similar to what happens with npm start. Are you interested in working on this part? The fastest way is to bring in a test dependency, some discussion here may help. If not, we can comment out the demo test for now and adress them in another PR soon.

I’ve marked the areas to address this in the diff below:

diff --git a/test/demo.js b/test/demo.js
index 077feba..d528be5 100644
--- a/test/demo.js
+++ b/test/demo.js
@@ -23,6 +23,7 @@ const demos = [

 (async () => {
     before(async function() {
+        // TODO: serve site via server here
         global.browser = global.browser || await puppeteer.launch();
     });

@@ -68,5 +69,8 @@ const demos = [
         });
     });

-    after(async () => browser.close());
+    after(async () => {
+        // TODO: close server here
+        browser.close();
+    });
 })();
\ No newline at end of file
voidCounter commented 2 months ago

@shakeelmohamed Thank you! Will work on this.

voidCounter commented 2 months ago

@shakeelmohamed

  1. Ran the site through a server using http-server during the demo test. I was still getting the timeout issue.
  2. mocha has a default timeout value of 2000ms. But the demo takes >2000ms during testing. So, increasing the timeout value resolves the issue.
    // package.json
    "test": "mocha --timeout 10000",

    By the way, using the new headless implementation during launching the browser instance through headless: "new" as Puppeteer suggested, gives me an assert failure during assert.notEqual(oldUrl, page.url());. But the old implementation works fine. image

shakeelmohamed commented 2 months ago

@voidCounter Approved and merged, really great work here! Thank you for going through multiple rounds of feedback and for some additional considerations. I would love to see future contributions from you if you’re interested.

voidCounter commented 2 months ago

@shakeelmohamed Thank you so much for your guidance throughout the commits! Learned a lot.