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

Add Puppeteer for End-to-End Tests #567

Merged
merged 24 commits into from
Feb 1, 2024
Merged

Add Puppeteer for End-to-End Tests #567

merged 24 commits into from
Feb 1, 2024

Conversation

garrettmflynn
Copy link
Member

This PR adds puppeteer to our stack to enable end-to-end tests. The current code implements a basic check for the number of pipelines (should always be zero because the test is using a scoped home folder that is wiped each time), then takes a screenshot of the home page.

I'm open to whatever additions to this PR before merging so that it's useful. I thought we might implement a full E2E test using the GIN testing data—which would also allow us to auto-generate screenshots for our new tutorial—but I wanted to confirm the best way to introduce these features.

@garrettmflynn garrettmflynn self-assigned this Jan 17, 2024
@CodyCBakerPhD
Copy link
Collaborator

I thought we might implement a full E2E test using the GIN testing data—which would also allow us to auto-generate screenshots for our new tutorial—but I wanted to confirm the best way to introduce these features.

That would be awesome quite honestly, both for perfect testing assurance as well as keeping documentation up-to-date

Could you try that in a splinter branch follow-up PR that uses this one as a base?

@CodyCBakerPhD
Copy link
Collaborator

Note for getting the CI to have the test data, I'd follow the approach of NeuroConv, where a new workflow handles download and caching per platform by downloading from the aws copies (I will probably need to set some secrets for this however): https://github.com/catalystneuro/neuroconv/blob/main/.github/workflows/testing.yml#L113-L127

@CodyCBakerPhD
Copy link
Collaborator

Huh, pretty cool how easy that looks to setup/configure!

The CI seems to complain about not connecting to the port however

@CodyCBakerPhD
Copy link
Collaborator

TODO: Garrett is investigating a self-hosted runner for this since GitHub Actions seems to have problems with headless state?

@garrettmflynn
Copy link
Member Author

garrettmflynn commented Jan 31, 2024

Printed out all the messages from Electron and actually getting somewhere now!

However, there are different behaviors for each operating system...

Linux

Does not support display, so closes Electron on start.

stderr | unknown test
[electron] [2333:0131/181612.200458:ERROR:ozone_platform_x11.cc(241)] Missing X server or $DISPLAY

[electron] [2333:0131/181612.201710:ERROR:env.cc(255)] The platform failed to initialize.  Exiting.


stdout | unknown test
<empty line>
stderr | unknown test
<empty line>
stdout | unknown test
[electron] Exited with code 0

 ✓ tests/metadata.test.ts  (5 tests) [130](https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/7730543101/job/21076184217#step:10:131)9ms
 ❯ tests/e2e.test.ts  (1 test) 10474ms

⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  tests/e2e.test.ts > E2E Test
Error: net::ERR_CONNECTION_REFUSED at http://localhost:8315
 ❯ navigate node_modules/puppeteer/node_modules/puppeteer-core/src/cdp/Frame.ts:191:13
 ❯ Function.race node_modules/puppeteer/node_modules/puppeteer-core/src/util/Deferred.ts:44:14
 ❯ CdpFrame.goto node_modules/puppeteer/node_modules/puppeteer-core/src/cdp/Frame.ts:144:17
 ❯ CdpPage.goto node_modules/puppeteer/node_modules/puppeteer-core/src/api/Page.ts:1609:12
 ❯ tests/puppeteer.ts:50:5
     48|     const browser = output.browser = await puppeteer.launch({ headless…
     49|     const page = output.page = await browser.newPage();
     50|     await page.goto(browserURL);
       |     ^
     51|     const endpoint = await page.evaluate(() => fetch(`json/version`).t…
     52|     await browser.close()

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed | 2 passed (3)
      Tests  9 passed (10)
   Start at  18:16:07
   Duration  12.34s (transform 804ms, setup 2ms, collect 3.64s, tests 11.79s, environment 1.91s, prepare 480ms)

Mac

Is just timing out? Looks like there are some GPU issues, but nothing seems to be failing...

[electron] [3507:0131/181811.905793:ERROR:command_buffer_proxy_impl.cc(128)] ContextResult::kTransientFailure: Failed to send GpuControl.CreateCommandBuffer.
<empty line>

stdout | unknown test

<empty line>
stderr | unknown test
[electron] [3509:0131/181811.907841:ERROR:command_buffer_proxy_impl.cc([128](https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/7730543101/job/21076184500#step:10:129))] ContextResult::kTransientFailure: Failed to send GpuControl.CreateCommandBuffer.


stderr | unknown test
stdout | unknown test
[electron] [2024-01-31 18:18:12,394] INFO in app: Starting server on port 4242
[electron]  * Serving Flask app 'app'

 * Debug mode: off





stderr | unknown test
stdout | unknown test
[electron] WARNING: This is a development server. Do not use it in a production deployment. Use a production WSGI server instead.
<empty line>
 * Running on http://127.0.0.1:4242
Press CTRL+C to quit



stderr | unknown test
stdout | unknown test
[electron] 127.0.0.1 - - [31/Jan/2024 18:18:14] "GET /startup/echo?arg=server%20ready HTTP/1.1" 200 -
<empty line>



 ❯ tests/e2e.test.ts  (1 test) 11268ms
⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯


 Test Files  1 failed | 2 passed (3)
 FAIL  tests/e2e.test.ts > E2E Test
      Tests  9 passed (10)
Error: Hook timed out in 11000ms.
   Start at  18:18:02
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
   Duration  13.93s (transform 699ms, setup 0ms, collect 5.54s, tests 13.31s, environment 2.56s, prepare 378ms)

Windows

This one doesn't even seem to start. Doing some more logging to see what's happening under the hood.

⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  tests/e2e.test.ts > E2E Test
Error: Hook timed out in 11000ms.
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯ Uncaught Exception ⎯⎯⎯⎯⎯
Error: spawn npm ENOENT
 ❯ Process.ChildProcess._handle.onexit node:internal/child_process:283:19
 ❯ onErrorNT node:internal/child_process:476:16
 ❯ processTicksAndRejections node:internal/process/task_queues:82:21

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -4058, code: 'ENOENT', syscall: 'spawn npm', path: 'npm', spawnargs: [ 'run', 'start' ] }
This error originated in "tests/e2e.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 Test Files  1 failed | 2 passed (3)
      Tests  9 passed (10)
     Errors  1 error
   Start at  18:22:42
   Duration  13.33s (transform 742ms, setup 0ms, collect 4.43s, tests 12.41s, environment 2.[38](https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/7730543101/job/21076184742#step:10:39)s, prepare [59](https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/7730543101/job/21076184742#step:10:60)0ms)

@garrettmflynn
Copy link
Member Author

@CodyCBakerPhD Mac is working.

Will have to play around with Windows / Linux to see if I can get past these issues. Would you mind running these tests locally on your PC to see if it works?

@garrettmflynn
Copy link
Member Author

Fixed Ubuntu using the suggestions in microsoft/playwright#11932

@CodyCBakerPhD
Copy link
Collaborator

Great news; I'd be happy to start off with this running on only a single (or now I suppose two, since you got it) platform(s) and only adding more if easy to do

@CodyCBakerPhD
Copy link
Collaborator

When I run locally on windows, the tests start fine and the app even launches, then leads to

image

with

 ✓ tests/progress.test.ts (4)
 ❯ tests/e2e.test.ts (1) 11113ms
   ❯ E2E Test (1) 11113ms
     - [ beforeAll ]
     · Ensure number of test pipelines starts at zero
 ✓ tests/metadata.test.ts (5) 1223ms

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  tests/e2e.test.ts > E2E Test
Error: Hook timed out in 11000ms.
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed | 2 passed (3)
      Tests  9 passed (10)
   Start at  14:13:16
   Duration  12.74s (transform 961ms, setup 0ms, collect 4.01s, tests 12.34s, environment 1.78s, prepare 441ms)

in the console

@CodyCBakerPhD
Copy link
Collaborator

But if you'd rather deal with windows in a follow up, just let me know, and configure this PR to only run the e2e test on a subset of the platforms

@garrettmflynn
Copy link
Member Author

Think I got it after testing locally myself.

Turns out the startup time for Electron is really slow on Windows (~40s for me until the window is ready). I've made the tests wait for a "ready" message from the Electron subprocess before beginning, otherwise will timeout if this takes longer than a minute to receive.

@garrettmflynn garrettmflynn marked this pull request as ready for review January 31, 2024 22:33
@garrettmflynn
Copy link
Member Author

garrettmflynn commented Jan 31, 2024

Probably ideal to merge this in as a basic integration of puppeteer, followed by two additional PRs:

  1. A complete end-to-end test
  2. Screenshot generation for the tutorial

Though these may end up being closely related.

Let me know if you'd like to proceed in a different way.

@CodyCBakerPhD CodyCBakerPhD enabled auto-merge (squash) February 1, 2024 16:07
@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn Incredible work! This is fantastic

Got it to work on my own system, but it took a couple of tries so maybe retrying is all it takes

@CodyCBakerPhD CodyCBakerPhD merged commit 42b8edf into main Feb 1, 2024
9 of 10 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the puppeteer branch February 1, 2024 16:14
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.

2 participants