Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add a crate to test the in-browser light client. #4887

Merged
merged 56 commits into from
Apr 20, 2020

Conversation

expenses
Copy link
Contributor

At the moment, the wasm light client is at a high risk of being broken by incoming commits. This PR adds a simple test that makes sure the light client connects to peers and starts syncing. This requires #4880 to me merged first.

It's pretty messy, but I'm not sure if there's a great way to improve that.

Copy link
Contributor

@tomaka tomaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course also waiting to switch to published libp2p.

bin/node/browser-testing/src/ws.js Outdated Show resolved Hide resolved
bin/node/browser-testing/src/lib.rs Outdated Show resolved Hide resolved
bin/node/browser-testing/src/lib.rs Outdated Show resolved Hide resolved
@TriplEight
Copy link
Contributor

Firefox will be added to the CI env in paritytech/scripts#152

@expenses
Copy link
Contributor Author

@tomaka Would you like to wait until libp2p/rust-libp2p#1454 is merged and libp2p 0.16.1 is published before merging this? I'm happy either way.

Copy link
Contributor

@tomaka tomaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, assuming you manage to make the test work.

bin/node/browser-testing/src/lib.rs Outdated Show resolved Hide resolved
bin/node/browser-testing/src/lib.rs Outdated Show resolved Hide resolved
@expenses
Copy link
Contributor Author

I've merged in #5613.

@expenses expenses requested a review from tomaka April 12, 2020 13:38
@bkchr
Copy link
Member

bkchr commented Apr 13, 2020

Still not working?

@expenses
Copy link
Contributor Author

Still not working?

It worked once: https://gitlab.parity.io/parity/substrate/-/jobs/459858, but then when i re-ran that test it failed, with the only different being the port number that wasm-pack-test-runner randomly selects (I haven't noticed a pattern about which ports work and which don't though).

@bkchr
Copy link
Member

bkchr commented Apr 14, 2020

Hmm :(

@bkchr
Copy link
Member

bkchr commented Apr 14, 2020

@expenses Did you tried running it with verbose option enabled to better debug this?

@gavofyork
Copy link
Member

@expenses ?

@TriplEight
Copy link
Contributor

currently, it doesn't work with the chromedriver, working on it.

@TriplEight
Copy link
Contributor

@expenses
tried with your new branch https://github.com/expenses/wasm-bindgen/tree/port-env-var with WASM_BINDGEN_TEST_ADDRESS set to 127.0.0.1:37351 (also with 9515).
It's reproducible with

docker run -it -v ~/cache/substrate:/ci-cache parity/rust-builder:latest
# and there 
export SCCACHE_DIR=/ci-cache/sccache
export CARGO_HOME=/ci-cache/cargo
export CARGO_TARGET_DIR=/ci-cache/targets/
export RUSTFLAGS="-Cdebug-assertions=y"
git clone https://github.com/paritytech/substrate .
git checkout ashley-browser-testing
export CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER="wasm-bindgen-test-runner"
export WASM_BINDGEN_TEST_TIMEOUT=60
cargo +nightly test --target wasm32-unknown-unknown -p node-browser-testing -Z features=itarget

and
webdriver.json

{"goog:chromeOptions": {
    "args": [
      "--verbose",
      "whitelisted-ips=127.0.0.1",
      "--headless",
      "--disable-gpu",
      "--no-sandbox"
    ]
  }
}

It did not work:

logs
    Finished test [unoptimized + debuginfo] target(s) in 0.47s
     Running /ci-cache/targets/wasm32-unknown-unknown/debug/deps/node_browser_testing-3ef5d09ca18e450f.wasm
Set timeout to 60 seconds...
Running headless tests in Chrome on `http://127.0.0.1:9515/`
Try find `webdriver.json` for configure browser's capabilities:
Ok
Failed to detect test as having been run. It might have timed out.
output div contained:
    running 1 test

driver status: signal: 9                          
driver stdout:
    Starting ChromeDriver 80.0.3987.162 (f2c5dd6138153bb0658091205bd1a1717f16081a-refs/branch-heads/3987@{#1034}) on port 9515
    Only local connections are allowed.
    Please protect ports used by ChromeDriver and related test frameworks to prevent access by malicious code.

driver stderr:
    [1587141083.320][SEVERE]: bind() failed: Cannot assign requested address (99)

Error: some tests failed                          
error: test failed, to rerun pass '-p node-browser-testing --lib'

# 
root@83b8c00b60ca:/builds# chromedriver --whitelisted-ips=127.0.0.1 --verbose
Starting ChromeDriver 80.0.3987.162 (f2c5dd6138153bb0658091205bd1a1717f16081a-refs/branch-heads/3987@{#1034}) on port 9515
Remote connections are allowed by a whitelist (127.0.0.1).
Please protect ports used by ChromeDriver and related test frameworks to prevent access by malicious code.

Apparently, this setup does not read webdriver.json even though it tells that finds it:

Try find `webdriver.json` for configure browser's capabilities:
Ok

Proof:

in cargo test run there's Only local connections are allowed. and if run chromedriver --whitelisted-ips=127.0.0.1 it runs with Remote connections are allowed by a whitelist (127.0.0.1).
Native chromedriver's port is 9515
I have no clue how to pass flags to it, so I stuck with debug here.

@expenses
Copy link
Contributor Author

@TriplEight Oh, thanks for figuring out how to make it reproducible!

@expenses
Copy link
Contributor Author

I think I've finally gotten it to work 😊😊😊😊

@gnunicorn gnunicorn merged commit 9207eda into master Apr 20, 2020
@gnunicorn gnunicorn deleted the ashley-browser-testing branch April 20, 2020 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants