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

[Flaky Tests]: Link Form Submission #633

Closed

Conversation

seanpdoyle
Copy link
Contributor

The test passes consistently locally, but fails consistently in CI:

1) [chrome] › form_submission_tests.ts:909:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when html[data-turbo=false]

  AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form'

    918 |   await link.click()

    919 |

  > 920 |   assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")

        |          ^

    921 |   assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")

    922 | })

    923 |

      at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:920:10

2) [chrome] › form_submission_tests.ts:924:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when Turbo.session.drive = false

  AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form'

    932 |   await link.click()

    933 |

  > 934 |   assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")

        |          ^

    935 |   assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")

    936 | })

    937 |

      at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:934:10

@seanpdoyle seanpdoyle force-pushed the ensure-drive-enabled-in-tests branch from a4a405f to 770aa23 Compare July 18, 2022 01:47
The test passes consistently locally, but fails consistently in CI:

```
1) [chrome] › form_submission_tests.ts:909:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when html[data-turbo=false]

  AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form'

    918 |   await link.click()

    919 |

  > 920 |   assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")

        |          ^

    921 |   assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")

    922 | })

    923 |

      at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:920:10

2) [chrome] › form_submission_tests.ts:924:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when Turbo.session.drive = false

  AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form'

    932 |   await link.click()

    933 |

  > 934 |   assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")

        |          ^

    935 |   assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")

    936 | })

    937 |

      at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:934:10
```
@seanpdoyle seanpdoyle force-pushed the ensure-drive-enabled-in-tests branch from 770aa23 to 5aae196 Compare July 18, 2022 01:48
@seanpdoyle
Copy link
Contributor Author

Since this passed and reported the summary, I'm closing and including the commit in #479 and #431.

@seanpdoyle seanpdoyle closed this Jul 18, 2022
@seanpdoyle seanpdoyle deleted the ensure-drive-enabled-in-tests branch July 18, 2022 01:58
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 18, 2022
Follow-up to hotwired#633

The flaky test outlined in [hotwired#633][] was "flaky", but not
in the sense that was originally suspected. Somehow, it was presenting
as a false negative, failing when we thought it should consistently
pass.

On further inspection, it was _passing when it should consistently
fail_.

This commit addresses the underlying issue by copying any
`[data-turbo-frame]` attributes onto the `<form>` element from the `<a>`
element that is clicked outside of the targeted `<turbo-frame>`.

[hotwired#633]: hotwired#633
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 18, 2022
Follow-up to hotwired#633

The flaky test outlined in [hotwired#633][] was "flaky", but not
in the sense that was originally suspected. Somehow, it was presenting
as a false negative, failing when we thought it should consistently
pass.

On further inspection, it was _passing when it should consistently
fail_.

This commit addresses the underlying issue by copying any
`[data-turbo-frame]` attributes onto the `<form>` element from the `<a>`
element that is clicked outside of the targeted `<turbo-frame>`.

[hotwired#633]: hotwired#633
dhh pushed a commit that referenced this pull request Jul 18, 2022
* Add tests for pausable Frame Rendering

Follow-up to #431

Support for the `turbo:before-frame-render` event relies on the same
underlying infrastructure as the `turbo:before-render` event (i.e. the
`Renderer` abstract class). As a result of re-using that abstraction,
listeners for the `turbo:before-frame-render` event can also leverage
the `detail.resume` function in the same way to handle asynchronous
rendering.

The changes made in [#431][] excluded test coverage for
that behavior. This commit adds that coverage to guard against
regressions in that behvaior.

[#431]: #431

* Form Links: Copy `[data-turbo-frame]` from `<a>` to `<form>`

Follow-up to #633

The flaky test outlined in [#633][] was "flaky", but not
in the sense that was originally suspected. Somehow, it was presenting
as a false negative, failing when we thought it should consistently
pass.

On further inspection, it was _passing when it should consistently
fail_.

This commit addresses the underlying issue by copying any
`[data-turbo-frame]` attributes onto the `<form>` element from the `<a>`
element that is clicked outside of the targeted `<turbo-frame>`.

[#633]: #633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant