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

JS tests are flaky #2549

Open
mgeisler opened this issue Jan 14, 2025 · 7 comments
Open

JS tests are flaky #2549

mgeisler opened this issue Jan 14, 2025 · 7 comments
Assignees

Comments

@mgeisler
Copy link
Collaborator

          Oh, this fails because of an unrelated timeout:
[0-1] Error in "Playground.executes the hello world code and prints the hello message"
Error: Expect  to have text

Expected: StringContaining "🌍"
Received: "The operation timed out: deadline has elapsed"
    at Context.<anonymous> (/home/runner/work/comprehensive-rust/comprehensive-rust/tests/src/playground.test.ts:[30](https://github.com/google/comprehensive-rust/actions/runs/12767680980/job/35586546795?pr=2548#step:15:31):37)
[0-1] Error in "Playground.shows error messages in stderr if the code is broken"
Error: Expect $(`code.result.stderr`) to be displayed

The new JS test is from #2513 (cc @michael-kerscher). We might need to disable it for now if it turns out to be flaky.

Originally posted by @mgeisler in #2548 (comment)

@max-heller
Copy link
Collaborator

Getting the same error on my latest commit merged to main

@mgeisler
Copy link
Collaborator Author

Thanks @max-heller, let's disable this test for now!

mgeisler added a commit that referenced this issue Jan 15, 2025
This should stop the test failures, but of course it won’t fix the
underlying problem.

Reverts part of #2513. Related to #2549.
@mgeisler mgeisler changed the title JS tests might be flaky JS tests are flaky Jan 15, 2025
mgeisler added a commit that referenced this issue Jan 15, 2025
This should stop the test failures, but of course it won’t fix the
underlying problem.

Reverts part of #2513. Related to #2549.
@michael-kerscher
Copy link
Collaborator

I'm not entirely sure yet how to proceed from here. I would like to test the playground, as this is an important bit of the course book - but I also don't want to have flaky tests that don't have anything to do with regular commits.
There would be some options though that I wanted to document and bring up for discussion:

  • increase timeouts for tests and/or some retry behavior (in production not test code)
  • detect if relevant files have been changed (e.g. filter for templates and javascript only). A change in the markdown part or translations should not modify the behavior of the html files. This limits impact of broken tests to relevant changes only.
  • mock the playground server https://webdriver.io/docs/mocksandspies/ in such a way that we don't need to rely on external dependencies
    Example on how to do this:
    // mock the response so this test does not rely on the playground backend to be working
    const playground_mock = await browser.mock(
      "https://play.rust-lang.org/execute",
      { method: "post" }
    );

    // be aware that there is a still a preflight OPTIONS request
    playground_mock.respond(
      {
        success: true,
        exitDetail: "Exited with status 0",
        stdout: "Hello 🌍!\n",
        stderr:
          "   Compiling playground v0.0.1 (/playground)\n    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.62s\n     Running `target/debug/playground`\n",
      },
      {
        headers: {
          "Access-Control-Allow-Origin": "*",
        },
        fetchResponse: false,
      }
    );

@mgeisler
Copy link
Collaborator Author

  • increase timeouts for tests and/or some retry behavior (in production not test code)

That would be my first suggestion too. If there are some timeouts we can adjust across the board, then we should do that. If there are retries we can enable easily, then we should do that too.

@michael-kerscher
Copy link
Collaborator

Regarding timeouts, there is our own 15 second timeout:

function fetch_with_timeout(url, options, timeout = 15000) {

But there are also HTTP 500 results from the playground server with this body:

{"error":"The operation timed out: deadline has elapsed"}

I also noticed that if the second variant occurs, this can occur multiple times - so retrying is not likely to succeed...

We could try to determine if we get any of the above error situations in the tests and ignore them explicitly - these results are most likely not due to issues on our side so our test infrastructure should not fail (but log warnings)

@djmitche
Copy link
Collaborator

Is it possible to adjust the browser automation to intercept the fetch request to the playground and return a value without actually interacting with the playground server?

@michael-kerscher
Copy link
Collaborator

that is the mocking approach that I mentioned above. Unfortunately I noticed that fetchResponse: false is documented but has no effect. I created a bug in webdriverio for that - but nevertheless it could overwrite the HTTP 500 - it does wait for the actual request to be finished currently. There is also a bug regarding using unicode that is currently worked on. So yes, something like that will be possible and useful quite soon.

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

No branches or pull requests

4 participants