feat: electron 17 and playwright tests #1937

hacdias · 2021-12-06T15:31:23Z

Closes #1902

hacdias · 2021-12-06T15:39:17Z

src/index.js

+if (process.env.NODE_ENV === 'test') {
+  const path = require('path')
+
+  app.setPath('home', process.env.HOME)
+  app.setPath('userData', path.join(process.env.HOME, 'data'))
+}


Previously, this was somewhat on the include.js file that we'd require using the flag -r when booting Electron. However, for some reason, even though we used setPath('userData', ...) there, the value wouldn't be reflected after the actual application started. Thus, I found this to be the best solution.

hacdias · 2021-12-06T15:39:47Z

src/daemon/index.js

@@ -63,7 +64,7 @@ module.exports = async function (ctx) {
      }

      log.end()
-      updateStatus(STATUS.STARTING_FINISHED)
+      updateStatus(STATUS.STARTING_FINISHED, id)


We're not scraping the logs anymore, so we send the peer id on the event once the startup is finished.

hacdias · 2021-12-06T15:40:23Z

@lidel the move is not as straightforward as I thought. Tests are failing on Ubuntu and partially failing on Windows. I will take a look on this Friday when I have more time. I left some code annotations on why I changed some things.

hacdias · 2021-12-10T14:40:19Z

Update: I will work on the Linux and Windows testing. I have to setup some VMs so it'll take a bit longer.

Update 2: all tests pass locally on a Ubuntu installation. Hmm...

Update 3: Windows working. It was just missing setting IPFS_PATH as an environment variable as we did before. Overriding all variables on Windows is an hassle. Maybe in the future we can look into how to properly replace %APPDATA% and %USERPROFILE% in tests. This way, we can add a test where we don't explicitly set the IPFS_PATH such that we can verify if the repository is created on the correct place.

I think Linux is related to the test setup, as it works locally on an Ubuntu VM.

hacdias · 2021-12-13T14:27:20Z

@lidel could you please take a look into the Ubuntu tests? They all time out but I can't reproduce the problem.

hacdias · 2021-12-17T15:34:11Z

Update: managed to reproduce on an Ubuntu server instance.

test/e2e/launch.e2e.js

lidel

Thank you for pushing here ❤️
Sadly, I've run out of time this week – will get back to this in Jan.

hacdias · 2022-01-03T13:34:59Z

Just a note: the tests on macOS on the CI seem unstable, randomly failing because with the error electronApplication.close: Browser closed.. I cannot reproduce that error locally and didn't figure out yet why is it happening. Maybe microsoft/playwright#5130?

hacdias · 2022-01-24T09:48:33Z

On #1961, for Electron 15.x version, we get a more specific error:

     electron.launch: Unable to open X display!
================================
Most likely this is because there is no X server available.
Use 'xvfb-run' on Linux to launch your tests with an emulated display server.
For example: 'xvfb-run npm run test:e2e'
================================

So it's definitely related to the xvfb thingy.

hacdias · 2022-01-24T10:47:43Z

I tried different combinations of commands to run xvfb, but none of them seems to work correctly. Perhaps updating the Xvfb action would fix whatever issue is happening. When I try to update it, it says that this repository can only run very specific versions of actions. I don't have permissions to update that and check.

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

This reverts commit b67a9f5.

this also solves the bug on Linux where electron.launch blocked for unknown reason – we were not passing process.env correctly

\o/

lidel

Thank you @hacdias for doing the heavy lifting, and apologies for being slow on this one ❤️

Poked at the CI issue, and it turned out to be caused by the meaning of env parameter passed to electron.launch – details inline.

CI is green, I switched to Electron 17 and works as expected, merging! 🚀

lidel · 2022-02-04T01:17:04Z

.github/workflows/ci.yml

@@ -100,7 +100,7 @@ jobs:
        run: npm run test

      - name: Test end-to-end
-        uses: GabrielBB/xvfb-action@f040be23a619e5ec34116f24098ad3626ceab681 # v1.4
+        uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6


ℹ️ This was not impacting build (i had the same timeout on the local Linux box), but I bumped version and safelisted this third-party action as part of investigation.

lidel · 2022-02-04T01:18:10Z

package.json

@@ -9,7 +9,7 @@
    "start": "cross-env NODE_ENV=development electron .",
    "lint": "standard",
    "test": "cross-env NODE_ENV=test mocha test/unit/**/*.spec.js",
-    "test:e2e": "xvfb-maybe cross-env NODE_ENV=test LIBP2P_ALLOW_WEAK_RSA_KEYS=1 mocha test/e2e/**/*.e2e.js --exit",
+    "test:e2e": "xvfb-maybe cross-env NODE_ENV=test playwright test test/e2e/launch.e2e.test.js",


ℹ️ switched to runner native to playright (just to avoid any bugs caused by legacy mocha stack)

lidel · 2022-02-04T01:19:30Z

package.json

    "cross-env": "^7.0.3",
    "delay": "^5.0.0",
    "dirty-chai": "^2.0.1",
    "dotenv": "^14.2.0",
-    "electron": "13.6.7",
+    "electron": "^17.0.0",


lidel · 2022-02-04T01:27:58Z

test/e2e/launch.e2e.test.js

+      env: Object.assign({}, process.env, {
        NODE_ENV: 'test',
        HOME: home,
        IPFS_PATH: repoPath
-      }
+      })


ℹ️ @hacdias this fixed the weird CI failure on Linux.

It was caused by the difference in how env field works here.
In playwright, passed env fully replaces process.env for greater control, so we were missing some variables created by X11/Xvfb. This restores them, solving e2 on Linux.

That makes sense. Thanks for figuring it out :)

lidel · 2022-02-04T01:28:29Z

test/e2e/launch.e2e.test.js

-          return { peerId }
+  async function daemonReady (app) {
+    const peerId = await app.evaluate(async ({ ipcMain }) => new Promise((resolve, reject) => {
+      ipcMain.on('ipfsd', (status, peerId) => {


ℹ️ really cool, switching to more sane way of reading ipfsd status makes tests run really fast now!

lidel · 2022-02-04T01:29:37Z

test/e2e/launch.e2e.test.js

  }

-  it('creates a repository on startup', async function () {
+  test('creates a repository on startup', async () => {


ℹ️ this and below changes are just refactor from mocha and chai to modern @playwright/test

hacdias commented Dec 6, 2021

View reviewed changes

hacdias force-pushed the use-playwright branch from 5e6753f to 9185743 Compare December 13, 2021 12:22

hacdias requested a review from lidel December 13, 2021 14:26

hacdias force-pushed the use-playwright branch from 956c081 to 409ba68 Compare December 17, 2021 11:57

hacdias mentioned this pull request Dec 17, 2021

chore: update ipfs deps #1899

Merged

hacdias commented Dec 17, 2021

View reviewed changes

test/e2e/launch.e2e.js Show resolved Hide resolved

hacdias commented Dec 17, 2021

View reviewed changes

test/e2e/launch.e2e.js Show resolved Hide resolved

lidel reviewed Dec 17, 2021

View reviewed changes

lidel self-requested a review December 17, 2021 23:46

hacdias force-pushed the use-playwright branch from bcba482 to 567b5eb Compare January 24, 2022 09:21

hacdias force-pushed the use-playwright branch 2 times, most recently from 3e86eba to d17b338 Compare January 24, 2022 10:46

hacdias force-pushed the use-playwright branch from f217053 to d17b338 Compare January 28, 2022 12:31

lidel mentioned this pull request Feb 3, 2022

feat: electron 16 #1961

Closed

lidel changed the title ~~feat: move to playwright~~ feat: electron 16 and playwright tests Feb 3, 2022

lidel changed the title ~~feat: electron 16 and playwright tests~~ feat: electron 17 and playwright tests Feb 4, 2022

hacdias added 5 commits February 4, 2022 02:14

feat: move to playwright

19569a6

fix: set repoPath as we did before

914e695

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

wip: remove timeout to see actual error

b46b9ac

wip: put mocha timeout back

2599f03

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

wip: replace isRunning by evt handler

20b85eb

hacdias and others added 7 commits February 4, 2022 02:14

Revert "wip: replace isRunning by evt handler"

71bde8a

This reverts commit b67a9f5.

wip: do not stop ipfs since closing the app already stops ipfs

aa781d6

rm

d3e03d1

update mocha and playwright

ebb986d

chore: GabrielBB/xvfb-action v1.6

ac7a867

refactor: switch e2e to @playwright/test

46b6ac0

this also solves the bug on Linux where electron.launch blocked for unknown reason – we were not passing process.env correctly

feat: electron 17

7fe8667

\o/

lidel force-pushed the use-playwright branch from 0505a04 to 7fe8667 Compare February 4, 2022 01:15

lidel approved these changes Feb 4, 2022

View reviewed changes

lidel merged commit 0669286 into main Feb 4, 2022

lidel deleted the use-playwright branch February 4, 2022 01:57

This was referenced Feb 4, 2022

proof of concept: close Web UI window instead of hiding #1929

Closed

E2E test setup filecoin-station/desktop#5

Closed

jameskerr mentioned this pull request Feb 17, 2022

View Log Detail with Inspector brimdata/zui#2184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: electron 17 and playwright tests #1937

feat: electron 17 and playwright tests #1937

hacdias commented Dec 6, 2021 •

edited by lidel

Loading

hacdias Dec 6, 2021

hacdias Dec 6, 2021

hacdias commented Dec 6, 2021

hacdias commented Dec 10, 2021 •

edited

Loading

hacdias commented Dec 13, 2021

hacdias commented Dec 17, 2021

lidel left a comment

hacdias commented Jan 3, 2022 •

edited

Loading

hacdias commented Jan 24, 2022 •

edited

Loading

hacdias commented Jan 24, 2022

lidel left a comment

lidel Feb 4, 2022 •

edited

Loading

lidel Feb 4, 2022

lidel Feb 4, 2022

lidel Feb 4, 2022

hacdias Feb 6, 2022

lidel Feb 4, 2022

lidel Feb 4, 2022

feat: electron 17 and playwright tests #1937

feat: electron 17 and playwright tests #1937

Conversation

hacdias commented Dec 6, 2021 • edited by lidel Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacdias commented Dec 6, 2021

hacdias commented Dec 10, 2021 • edited Loading

hacdias commented Dec 13, 2021

hacdias commented Dec 17, 2021

lidel left a comment

Choose a reason for hiding this comment

hacdias commented Jan 3, 2022 • edited Loading

hacdias commented Jan 24, 2022 • edited Loading

hacdias commented Jan 24, 2022

lidel left a comment

Choose a reason for hiding this comment

lidel Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacdias commented Dec 6, 2021 •

edited by lidel

Loading

hacdias commented Dec 10, 2021 •

edited

Loading

hacdias commented Jan 3, 2022 •

edited

Loading

hacdias commented Jan 24, 2022 •

edited

Loading

lidel Feb 4, 2022 •

edited

Loading