Skip to content

Fix flaky tests #10743

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 4, 2025
Merged

Fix flaky tests #10743

merged 1 commit into from
Mar 4, 2025

Conversation

@eth3lbert
Copy link
Contributor

@eth3lbert eth3lbert commented Mar 3, 2025

Attempting to fix the following flaky tests:

https://github.com/rust-lang/crates.io/actions/runs/13632195641/job/38102274110?pr=10740#step:6:239

not ok 156 Chrome 133.0 - [147 ms] - Exam Partition 1 - Acceptance | support > reporting a crate from crate page: valid form without detail
    ---
        stack: >
            Error: Element not found when calling `click('[data-test-id="link-crate-report"]')`.
                at eval (webpack://crates-io/../../.pnpm/@[email protected]_@[email protected][email protected]_@[email protected][email protected][email protected]_/node_modules/@ember/test-helpers/dist/dom/click.js?:107:13)
                at async prepare (webpack://crates-io/./tests/acceptance/support-test.js?:3035:7)
                at async Object.eval (webpack://crates-io/./tests/acceptance/support-test.js?:3091:7)
        message: >
            Promise rejected during "valid form without detail": Element not found when calling `click('[data-test-id="link-crate-report"]')`.
        negative: >
            false
        browser log: |
    ...

and
https://github.com/rust-lang/crates.io/actions/runs/13632195641/job/38102274110?pr=10740#step:6:254

not ok 159 Chrome 133.0 - [90 ms] - Exam Partition 1 - Acceptance | support > reporting a crate from crate page: valid form with required detail
    ---
        stack: >
            Error: Element not found when calling `click('[data-test-id="link-crate-report"]')`.
                at eval (webpack://crates-io/../../.pnpm/@[email protected]_@[email protected][email protected]_@[email protected][email protected][email protected]_/node_modules/@ember/test-helpers/dist/dom/click.js?:107:13)
                at async prepare (webpack://crates-io/./tests/acceptance/support-test.js?:3035:7)
                at async Object.eval (webpack://crates-io/./tests/acceptance/support-test.js?:3132:7)
        message: >
            Promise rejected during "valid form with required detail": Element not found when calling `click('[data-test-id="link-crate-report"]')`.
        negative: >
            false
        browser log: |
    ...

@eth3lbert eth3lbert added C-bug 🐞 Category: unintended, undesired behavior A-frontend 🐹 labels Mar 3, 2025
Turbo87
Turbo87 reviewed Mar 3, 2025
View reviewed changes
tests/acceptance/support-test.js Outdated
Comment on lines 208 to 209
assert.strictEqual(currentURL(), '/crates/nanomsg');
assert.dom('[data-test-id="crate-sidebar"]').exists();
Copy link
Member

@Turbo87 Turbo87 Mar 3, 2025

Choose a reason for hiding this comment

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

how would this fix it?

Copy link
Contributor Author

@eth3lbert eth3lbert Mar 3, 2025

Choose a reason for hiding this comment

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

Ah, wait, sorry. I should have used waitFor rather than assert.dom 🤦.

Copy link
Contributor Author

@eth3lbert eth3lbert Mar 3, 2025

Choose a reason for hiding this comment

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

I just changed it to wait for the button to exist.

Copy link
Member

@Turbo87 Turbo87 Mar 4, 2025

Choose a reason for hiding this comment

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

I should have used waitFor rather than assert.dom 🤦.

I guess that should work, though I'm wondering why it is needed here. iirc the link-crate-report button should be rendered directly and there are no network requests or other async things that would prevent that. feels like something else might be wrong.

ultimately, the assertion architecture of playwright where waitFor() is the default is probably better than the test waiter architecture of Ember.js 😅

Copy link
Contributor Author

@eth3lbert eth3lbert Mar 4, 2025

Choose a reason for hiding this comment

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

irc the link-crate-report button should be rendered directly and there are no network requests or other async things that would prevent that. . feels like something else might be wrong.

Yeah, I have no clue what's going on here either! It only depends on the crate (@crate.name), which should be resolved in the routes, but it's failing somehow for no obvious reason. This led me to wait for it explicitly as a naive solution. I should investigate the root cause in more detail in the future.

ultimately, the assertion architecture of playwright where waitFor() is the default is probably better than the test waiter architecture of Ember.js 😅

Indeed, the auto-retry is much more ergonomic.

Turbo87
Turbo87 approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

I guess we can merge this for now until we find a better solution (or migrate the whole codebase to use waitFor() instead :P)

@Turbo87 Turbo87 merged commit 3a1eb3d into rust-lang:main Mar 4, 2025
11 checks passed
@eth3lbert eth3lbert deleted the fix-flaky-tests branch March 4, 2025 09:34
@Turbo87
Copy link
Member

Turbo87 commented Mar 10, 2025

Error: waitFor timed out waiting for selector "[data-test-id="link-crate-report"]"

https://github.com/rust-lang/crates.io/actions/runs/13762342974/job/38481079622?pr=10794

@eth3lbert
Copy link
Contributor Author

eth3lbert commented Mar 10, 2025

Error: waitFor timed out waiting for selector "[data-test-id="link-crate-report"]"

https://github.com/rust-lang/crates.io/actions/runs/13762342974/job/38481079622?pr=10794

I'm not sure what the issue is here. It would be great if we could take a screenshot when it fails to find the DOM.

@eth3lbert eth3lbert mentioned this pull request Mar 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Turbo87 Turbo87 Turbo87 approved these changes

Assignees

No one assigned

Labels

A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eth3lbert @Turbo87