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

test: add more e2e tests for device request prompt #11185

Conversation

outofambit
Copy link

@outofambit outofambit commented Oct 17, 2023

What kind of change does this PR introduce?

Adds some more basic e2e tests for DeviceRequestPrompt. These tests don't involve actual bluetooth connection scenarios since those would be hard to scaffold, but these will exercise more of the code to validate basic functioning and a couple error states. These e2e tests must be run headed since the bluetooth chooser will not launch on headless chrome instances.

Did you add tests for your changes?

It's all tests :)

If relevant, did you update the documentation?

Summary

Building off of #11072 and #11159, this PR adds a few more e2e tests for DeviceRequestPrompt that can be run headed.

Does this PR introduce a breaking change?

Nope.

Other information
cc @thiagowfx

Bug: #11189

@google-cla
Copy link

google-cla bot commented Oct 17, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcf bot commented Oct 17, 2023

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@thiagowfx thiagowfx requested a review from OrKoN October 18, 2023 08:10
@thiagowfx
Copy link
Contributor

FYI: I just filed #11189 and back-linked all merged PRs so far to it, including this one.

@thiagowfx thiagowfx changed the title Add more e2e tests for device request prompt test: add more e2e tests for device request prompt Oct 18, 2023
Copy link
Collaborator

@Lightning00Blade Lightning00Blade left a comment

Choose a reason for hiding this comment

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

LGTM + Two minor comment before we can merge this.

test/src/device-request-prompt.spec.ts Outdated Show resolved Hide resolved
test/src/device-request-prompt.spec.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@Lightning00Blade Lightning00Blade left a comment

Choose a reason for hiding this comment

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

LGTM + The Expectations needed to be update and one JS issue fixes - put suggested fixes.
Other then that once CI passes I will merge.

test/TestExpectations.json Outdated Show resolved Hide resolved
test/src/device-request-prompt.spec.ts Outdated Show resolved Hide resolved
@OrKoN OrKoN force-pushed the outofambit/device-request-prompt-e2e branch from dd727f6 to 1c22c71 Compare October 20, 2023 20:20
@OrKoN OrKoN enabled auto-merge (squash) October 20, 2023 20:22
@OrKoN OrKoN disabled auto-merge October 21, 2023 08:20
@thiagowfx thiagowfx force-pushed the outofambit/device-request-prompt-e2e branch 2 times, most recently from 295db9d to 24f7fc3 Compare October 23, 2023 09:22
@thiagowfx thiagowfx enabled auto-merge (squash) October 23, 2023 09:22
@OrKoN OrKoN force-pushed the outofambit/device-request-prompt-e2e branch from 24f7fc3 to d6a53b7 Compare October 23, 2023 10:55
@OrKoN OrKoN enabled auto-merge (squash) October 24, 2023 10:37
@OrKoN OrKoN force-pushed the outofambit/device-request-prompt-e2e branch from d6a53b7 to fcc417c Compare October 24, 2023 10:37
@Lightning00Blade
Copy link
Collaborator

While investigating the timeout issue I found some problems:

  1. We also need WebBluetooth to be enabled
  2. I don't think we can run this in CI environments due to them not having Bluetooth adapter
    2.1. I ran it on my Cloud env and I get Bluetooth adapter not available. while locally I can see the
    pop up appearing

auto-merge was automatically disabled November 7, 2023 19:28

Head branch was pushed to by a user without write access

outofambit and others added 4 commits November 22, 2023 14:53
Co-authored-by: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>
@thiagowfx thiagowfx force-pushed the outofambit/device-request-prompt-e2e branch from cf0301f to f8374a7 Compare November 22, 2023 14:00
@thiagowfx thiagowfx force-pushed the outofambit/device-request-prompt-e2e branch from f8374a7 to 1787c26 Compare November 22, 2023 14:16
@OrKoN
Copy link
Collaborator

OrKoN commented Nov 22, 2023

@thiagowfx do you have a plan for this? It looks like the feature is not testable on GitHub actions because there are no Bluetooth devices there.

@Lightning00Blade
Copy link
Collaborator

Closing this as it seems it not possible to test with GitHub Actions.

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

Successfully merging this pull request may close these issues.

None yet

4 participants