test: Add webkit test script #1627

MarcoPolo · 2023-03-16T00:16:55Z

This fails on linux. Works on MacOS. I haven't investigated why we fail on linux, but it does block us testing webkit as part of the multidim-inderop tester.

MarcoPolo · 2023-03-16T04:46:27Z

Are we not using protocol/.github#487 in this repo?

achingbrain · 2023-03-17T08:56:48Z

Are we not using protocol/.github#487 in this repo?

No, this repo uses gated releases instead of releasing on every change to master: #1589

achingbrain · 2023-03-17T08:58:27Z

From the CI error it looks like the job just needs to install some extra deps?

╔══════════════════════════════════════════════════════╗
║ Host system is missing dependencies to run browsers. ║
║ Please install them with the following command:      ║
║                                                      ║
║     sudo npx playwright install-deps                 ║
║                                                      ║
║ Alternatively, use apt:                              ║
║     sudo apt-get install libvpx7\                    ║
║         libevent-2.1-7\                              ║
║         libopus0\                                    ║
║         libwoff1\                                    ║
║         libharfbuzz-icu0\                            ║
║         libgstreamer-plugins-base1.0-0\              ║
║         libgstreamer-gl1.0-0\                        ║
║         libhyphen0\                                  ║
║         libmanette-0.2-0\                            ║
║         libgles2\                                    ║
║         gstreamer1.0-libav                           ║
║                                                      ║
║ <3 Playwright Team                                   ║
╚══════════════════════════════════════════════════════╝]

achingbrain · 2023-03-17T11:28:35Z

Thinking about it a bit more, Webkit is Webkit so do we need to run it on both?

Running it on Linux means it'll pull the cached deps from the check job which'll be faster.

MarcoPolo · 2023-03-17T15:42:10Z

From the CI error it looks like the job just needs to install some extra deps?

Not the actual error. Just the error in CI. This has failed on me on a linux with the correct deps installed.

MarcoPolo · 2023-03-24T17:28:57Z

The failure is when we do something like this:

await crypto.subtle.deriveKey(
    { "name": "PBKDF2", "salt": new Uint8Array(16), "iterations": 32767, "hash": { "name": "SHA-256" } },
    await crypto.subtle.importKey('raw', new Uint8Array(), { "name": "PBKDF2" }, false, ['deriveKey', 'deriveBits']),
    { name: "AES-GCM", length: 128 }, true, ['encrypt'])

here: https://github.com/libp2p/js-libp2p-crypto/blob/e7bb8b26655af7d0d8a7387a2923c6c3929952fe/src/ciphers/aes-gcm.browser.ts#L35

MarcoPolo · 2023-03-24T18:24:58Z

Possibly related: https://bugs.webkit.org/show_bug.cgi?id=169468#c1

I'm guessing these are failing because there is no PBKDF2 implementation on GTK, and that once someone hooks it up things will start working.

Edit, nvm it seems like that has been done.

MarcoPolo · 2023-03-24T21:01:53Z

Aha! it turns out webkit+linux doesn't like it when you import a key with no bytes.

e.g. this works:

await crypto.subtle.deriveKey(
    { "name": "PBKDF2", "salt": new Uint8Array(16), "iterations": 32767, "hash": { "name": "SHA-256" } },
    await crypto.subtle.importKey('raw', new Uint8Array(1), { "name": "PBKDF2" }, false, ['deriveKey', 'deriveBits']),
    { name: "AES-GCM", length: 128 }, true, ['encrypt'])

Confusingly, this error happens when you try to use the imported key with deriveKey rather than when you importKey.

MarcoPolo · 2023-05-05T04:42:04Z

This is blocked on libp2p/js-libp2p-crypto#319

…t-webkit

src/autonat/index.ts

achingbrain · 2023-05-19T06:54:08Z

test/autonat/index.spec.ts

-      stream.sink.returns(Promise.resolve())
+
+      // stub autonat protocol stream
+      const stream = stubObject<Stream>({


Trying to run this branch locally I noticed something weird with the stubbing but only in WebKit, though I didn't get to the bottom of it.

We stub a bunch of peer responses in each test to simulate different scenarios - once a certain number of tests were being run, the stubbed return values started being from the wrong tests.

Needs more investigation really.

Yes. Hence this change. I'm not sure why exactly this fails on WebKit but seems to work on other browsers. I'm not sure overriding values is supported in Sinon or if it's a hack we do. The change here is to not override and instead be explicit in what gets stubbed vs set.

My guess is the other failures we're seeing is related to something like this.

Have you checked out the Sinon docs? I believe we’re using it as intended, no “hack” involved.

I think sinon is not involved here. This should just be setting values on the object here: https://github.com/ttarnowski/ts-sinon/blob/master/src/index.ts#L67. Maybe Proxy in WebKit has slightly different behavior? Maybe a bug since this only shows up in certain cases (e.g. run only this test is fine, but run two versions of this test fails).

Yeah, it’s really weird because we don’t reuse the stubbed instances between the tests that I can see 🤔

@maschad did you figure out what the issue was here around Proxy?

@MarcoPolo I don't think the issue was related to Proxy but rather the peer id fixtures, the relay server behaved as if it was creating reservations for peers that seemed to be reconnecting due to the tests reusing peer ids, we resolved this in this PR

MarcoPolo · 2023-05-30T21:37:51Z

@maschad CI is still failing in webkit fyi

maschad · 2023-05-31T01:31:18Z

@maschad CI is still failing in webkit fyi

Thanks @MarcoPolo I actually created an issue as I don't believe this failure is unique to our webkit tests.

It seems like #1781 is currently blocking CI, will debug.

MarcoPolo · 2023-05-31T16:38:27Z

Is this test more likely to fail on WebKit?

Can we handle the flaky test separately (maybe skip for now) and merge WebKit tests now? It would be good to make sure we don't break WebKit from now on, and it would be useful for WebRTC testing.

maschad · 2023-05-31T16:53:51Z

Is this test more likely to fail on WebKit?

It seems to fail consistently on Webkit based on the last few job runs.

Can we handle the flaky test separately (maybe skip for now) and merge WebKit tests now? It would be good to make sure we don't break WebKit from now on, and it would be useful for WebRTC testing.

Good suggestion, I pushed a commit with a skip for now.

MarcoPolo · 2023-05-31T16:55:06Z

If it fails consistently on WebKit, it doesn't sound like a flaky test to me. Does it fail locally when you run this on a linux box?

maschad · 2023-05-31T17:14:40Z

If it fails consistently on WebKit, it doesn't sound like a flaky test to me. Does it fail locally when you run this on a linux box?

Good suggestion for the linux vm, it wasn't failing on my local macos. It does fail consistently on linux with a stream reset error, but only when ran with the entire suite as opposed to when run in isolation or in it's own test block suite i.e. libp2p.upgrader. I suspect there is a socket which is not being closed in the previous tests which is causing this... investigating...

maschad · 2023-06-01T03:39:03Z

@MarcoPolo it turns out the nodes were not shutting down properly in some identify service tests.

@achingbrain I do notice that the said tests addressed in a78db93 intentionally close before the connection is established, but that results in this error in the logs.

WebSocket connection to 'ws://127.0.0.1:15001/p2p/12D3KooWHFKTMzwerBtsVmtz4ZZEQy2heafxzWw6wNn5PPYkBxJ5' failed: WebSocket is closed before the connection is established.

it's not too noisy but perhaps we should supress this?

Closes #1781

achingbrain · 2023-06-01T16:59:28Z

@MarcoPolo it turns out the nodes were not shutting down properly in some identify service tests.

That's a bit weird - the libp2p node is shut down in the afterEach for those tests.

maschad · 2023-06-01T19:22:34Z

That's a bit weird - the libp2p node is shut down in the afterEach for those tests.

That's a good point @achingbrain could it be that the afterEach is called after the failing test is run in some sort of race condition ? Either way I am not sure if this related directly to this PR but seems to be a testing environment issue where our tests hang from time to time

MarcoPolo changed the title ~~Add webkit test script~~ feat: Add webkit test script Mar 16, 2023

MarcoPolo force-pushed the marco/js-libp2p-test-webkit branch 2 times, most recently from a9e5026 to 5a00304 Compare March 20, 2023 21:24

MarcoPolo mentioned this pull request Mar 23, 2023

feat: Browser to Browser libp2p/js-libp2p-webrtc#90

Merged

4 tasks

MarcoPolo mentioned this pull request Mar 24, 2023

fix: deriveKey in webkit linux (workaround) libp2p/js-libp2p-crypto#313

Merged

MarcoPolo force-pushed the marco/js-libp2p-test-webkit branch 3 times, most recently from 102ecfd to 1fd8276 Compare April 12, 2023 00:54

MarcoPolo added 3 commits April 28, 2023 15:55

Add webkit test script

7d73f81

Add webkit test to CI

cfd5c4b

Install playwright deps

c3cde5e

MarcoPolo force-pushed the marco/js-libp2p-test-webkit branch from 1fd8276 to c3cde5e Compare April 28, 2023 22:55

achingbrain and others added 3 commits April 30, 2023 08:52

Merge branch 'master' into marco/js-libp2p-test-webkit

c77c006

Fix stream stubbing. We can't set the property value like this on safari

6cc9030

Bump @libp2p/crypto

5ed2146

MarcoPolo mentioned this pull request May 2, 2023

navigator.userAgent is unreliable in determining if the browser is webkit on linux libp2p/js-libp2p-crypto#318

Closed

MarcoPolo added 2 commits May 5, 2023 08:49

Update @libp2p/crypto

111ba6b

Merge branch 'master' into marco/js-libp2p-test-webkit

f288f82

maschad self-assigned this May 9, 2023

Merge remote-tracking branch 'origin/master' into marco/js-libp2p-tes…

bc177f5

…t-webkit

MarcoPolo changed the title ~~feat: Add webkit test script~~ test: Add webkit test script May 18, 2023

achingbrain requested changes May 19, 2023

View reviewed changes

maschad mentioned this pull request May 23, 2023

feat: added multiaddr formatter to logging libp2p/js-libp2p-logger#34

Merged

maschad added 2 commits May 22, 2023 19:09

Merge branch 'master' into marco/js-libp2p-test-webkit

36c38ba

refactor: updated formatters

2f0dde9

maschad requested a review from achingbrain May 23, 2023 02:16

Merge branch 'master' into marco/js-libp2p-test-webkit

3da460d

maschad added 2 commits May 31, 2023 11:49

Merge branch 'master' into marco/js-libp2p-test-webkit

ede58c6

test: skip flaky connect and disconnect event tests

3d6a67e

maschad added 2 commits May 31, 2023 12:15

deps: update logger to enable multiaddr formatting

8e43bb5

test: ensure that libp2p nodes are shutdown in identify service spec

a78db93

chore: linting fixes

5a24425

achingbrain and others added 4 commits June 2, 2023 15:01

chore: remove webkit fixes

c21177e

refactor: updated multiaddr formatting in logs

d29de4a

Merge branch 'master' into marco/js-libp2p-test-webkit

34cdc28

chore: edits for consistency

37044fa

achingbrain approved these changes Jun 6, 2023

View reviewed changes

achingbrain merged commit 14aa40f into master Jun 6, 2023

achingbrain deleted the marco/js-libp2p-test-webkit branch June 6, 2023 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Add webkit test script #1627

test: Add webkit test script #1627

MarcoPolo commented Mar 16, 2023

MarcoPolo commented Mar 16, 2023

achingbrain commented Mar 17, 2023

achingbrain commented Mar 17, 2023

achingbrain commented Mar 17, 2023

MarcoPolo commented Mar 17, 2023

MarcoPolo commented Mar 24, 2023 •

edited

Loading

MarcoPolo commented Mar 24, 2023 •

edited

Loading

MarcoPolo commented Mar 24, 2023

MarcoPolo commented May 5, 2023

achingbrain May 19, 2023

MarcoPolo May 19, 2023

achingbrain May 19, 2023

MarcoPolo May 19, 2023

achingbrain May 20, 2023

MarcoPolo Jun 7, 2023

maschad Jun 7, 2023

MarcoPolo commented May 30, 2023

maschad commented May 31, 2023 •

edited

Loading

MarcoPolo commented May 31, 2023

maschad commented May 31, 2023 •

edited

Loading

MarcoPolo commented May 31, 2023

maschad commented May 31, 2023 •

edited

Loading

maschad commented Jun 1, 2023 •

edited

Loading

achingbrain commented Jun 1, 2023

maschad commented Jun 1, 2023 •

edited

Loading

test: Add webkit test script #1627

test: Add webkit test script #1627

Conversation

MarcoPolo commented Mar 16, 2023

MarcoPolo commented Mar 16, 2023

achingbrain commented Mar 17, 2023

achingbrain commented Mar 17, 2023

achingbrain commented Mar 17, 2023

MarcoPolo commented Mar 17, 2023

MarcoPolo commented Mar 24, 2023 • edited Loading

MarcoPolo commented Mar 24, 2023 • edited Loading

MarcoPolo commented Mar 24, 2023

MarcoPolo commented May 5, 2023

achingbrain May 19, 2023

Choose a reason for hiding this comment

MarcoPolo May 19, 2023

Choose a reason for hiding this comment

achingbrain May 19, 2023

Choose a reason for hiding this comment

MarcoPolo May 19, 2023

Choose a reason for hiding this comment

achingbrain May 20, 2023

Choose a reason for hiding this comment

MarcoPolo Jun 7, 2023

Choose a reason for hiding this comment

maschad Jun 7, 2023

Choose a reason for hiding this comment

MarcoPolo commented May 30, 2023

maschad commented May 31, 2023 • edited Loading

MarcoPolo commented May 31, 2023

maschad commented May 31, 2023 • edited Loading

MarcoPolo commented May 31, 2023

maschad commented May 31, 2023 • edited Loading

maschad commented Jun 1, 2023 • edited Loading

achingbrain commented Jun 1, 2023

maschad commented Jun 1, 2023 • edited Loading

MarcoPolo commented Mar 24, 2023 •

edited

Loading

MarcoPolo commented Mar 24, 2023 •

edited

Loading

maschad commented May 31, 2023 •

edited

Loading

maschad commented May 31, 2023 •

edited

Loading

maschad commented May 31, 2023 •

edited

Loading

maschad commented Jun 1, 2023 •

edited

Loading

maschad commented Jun 1, 2023 •

edited

Loading