test-runner: Run tests isolated by default in headless mode

[zvolin]

Hi. I wanted to propose some changes to the wasm test runner. I'm currently working on some internal projects that are using yew frontend framework and I came across some problems with the current state of test runner. I'm trying to address those by this PR but please notice that I'm only focusing on running frontend tests in headless browser. I may be missing some points about other forms of testing that this tool has to offer as I simply haven't used it in other ways. I tried to not modify the behavior of any other forms of testing. If you think that this proposal could be expanded also to deno / node tests, I can try to handle this either in scope of this PR or in a future one.

Problem

Currently running a binary containing wasm_bindgen_tests results in creating a js script that executes all tests sequentially one after another in a headless browser. This doesn't introduce any isolation of environment in which tests are run, that means that each test is impacting all tests that are executed after it. Consider this simple example:
Assume we are testing some website which has such content:

<a id="login" href="/login">Log in</a>
<a id="signup" href="/signup">Sign up</a>

And some tests (pseudocode warning):

#[wasm_bindgen_test]
fn webpage_should_allow_user_to_sign_up_or_log_in() {
    somehow_render(MyComponent);
    assert!(body().get_element_by_id("login").is_some());
    assert!(body().get_element_by_id("signup").is_some());
}

#[wasm_bindgen_test]
fn login_button_should_redirect_to_login_page() {
    somehow_render(MyComponent);
    let login: HtmlElement = body().get_element_by_id("login").unwrap().unchecked_into();
    login.click();
    assert!(body().inner_html().contains("Please provide password");
}

Those test cases looks pretty deterministic, however the test results will not be. We possibly already redirected to some other page and we rendered MyComponent second time, so depending on the implementation of somehow_render it may lead to some hidden complications.

Solution

Of course being aware of this behavior we can try to include some forms of cleanup in tests. In this (and possibly other) simple cases this can not be a problem. Eg. redirect back, remove inserted elements and so on. However restoring DOM to original state with all default listeners is not that simple, and restoring JavaScript state seems pretty impossible. We can have some Promises running in the void, some mutated global state etc.

With this in mind, I thought that the only possible way to make running such tests deterministic is to run a clean webdriver session for each test case. This seems to be the simplest (and only?) solution that delivers.

Problems of solution :)

Running a new webdriver session is a huge time overhead. When tests are run sequentially, it is often orders of magnitude slower than just executing test and we pay for it with each running testcase. The obvious solution for this is to make all tests execute in parallel. I've implemented this and it seems to be doing a good job shortening the time the tests are ran.

The implementation is probably not the smartest one, as I don't only run a webdriver for each of test cases but also a separated rouille server. This doesn't seem to be that heavy overhead but it is definitely also not the lightest in terms of resources. I know that this could be handled by only spawning the server once and modifying the requests that we make to the server, however I decided to go with the simplest thing right now. Implementation uses rayon to manage resources as I didn't want to bother with manual calculations of how many threads to spawn depending on available cores and so on. Rayon handles all of this by itself.

In my opinion the resources during testing is not that much of a problem this days as the tests execution time, so I consider this as a reasonable tradeoff.

Environment configuration

In this proposal, I've included running headless tests isolated by default. It can be controlled by NO_ISOLATED flag. I think that this should be a default behavior, as it makes tests deterministic. Also I've limited the output printed by wasm-bindgen-test-runner by hiding most of it's prints not directly related to ran tests behind the debugging flag. I renamed it from WASM_BINDGEN_NO_DEBUG to WASM_BINDGEN_DEBUG and it means that debug will be off by default. I think this is a reasonable to print only an absolute minimum when everything goes well.

Breaking API

I consider this changes as breaking api so I think if this is merged we should release a new version of wasm-bindgen with a note about those changes. Those are also worth documenting in official book or wherever you think they should however I haven't bothered in providing anything yet as I'm not sure what kind of response this will get.

Testing done

I've tested this PR on my project containing a handful of wasm tests, where I've driven some testcases to either fail or timeout and everything was working smoothly. I tried running tests of wasm-bindgen-cli itself and it seems to pass some tests and then hang somewhere unless I pass NO_ISOLATED=1. I haven't investigated it yet but I'll try to take care of this in the near future.

[alexcrichton]

Thanks for the suggestions here, but I'm not sure that this is something I can take on and help maintain. The testing support in wasm-bindgen is pretty standalone right now to the extent that having a "third party" external crate should work as well as the #[wasm_bindgen_test] macro is today. In that sense I think it would be fine to have a separate crate for this style of integration testing with parallel webdrivers.

Otherwise though I do realize there's a lot of deficiencies in the current wasm_bindgen_test infrastructure, it was sort of only ever intended to be a stop-gap to something else. This is a big chunk of new code to maintain though and I'm not sure I can do that

[zvolin]

Hi, thanks for the reply. Fair enough, I took that answer to the consideration. I wish I could say that I'd maintain this however I've never maintained anything open source yet and I've just started developing my first ever crate so I can't be sure that I'll find myself in that.

In that sense I think it would be fine to have a separate crate for this style of integration testing with parallel webdrivers.

I think I can try to take inspiration from you or just fork test and test-macro and see whether I can build something similar for my use case. Feel free to close this pr if you want and best of luck :)

[daxpedda]

I think one obvious optimization here would be to spawn a new window instead of a new Webdriver session for each test (I am totally clueless about Webdriver btw).

Would be actually happy to review and maintain it though, but I don't see myself agreeing on the basic premise. Because the problematic behavior is actually also how Cargo tests on native work, which isn't exactly ideal, but imo we should keep consistent with Cargo. See rust-lang/rust#47506 and rust-lang/rust#67650. So if Cargo ever implements a way to run tests in separate processes, we should follow suit.

The alternative here is the same as on native, just put tests in separate files.