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

feat: support device name in playwright fixtures #1472

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented Feb 3, 2024

Reference #1369
Reference #939

@yury-s
Copy link
Member Author

yury-s commented Feb 3, 2024

cc @uchagani

@@ -83,6 +84,15 @@ public Options setBrowserName(String browserName) {
return this;
}

public String getDeviceName() {
Copy link

Choose a reason for hiding this comment

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

It's unfortunate that we cannot read back the actual values here, only the device name. However, that's better than nothing.

@yury-s yury-s merged commit 7ce6c7f into microsoft:main Feb 5, 2024
16 of 19 checks passed
public String defaultBrowserType;

static DeviceDescriptor findByName(Playwright playwright, String name) {
JsonArray devices = ((PlaywrightImpl) playwright).deviceDescriptors();
Copy link
Contributor

Choose a reason for hiding this comment

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

@yury-s this is returning null for me and so a null pointer exception is being thrown.

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, that's odd. Is options in options.deviceName null or where does it come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe PlaywrightImpl.deviceDescriptors() is returning null so the for loop throws a null pointer exception:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, do you have a test where this is happening? This should never happen if playwright is initialized.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I assume it is just the new test. It passes on the bots and locally. Could it be that you have some stale driver? We changed the way device descriptors pushed to the client some time ago. Try downloading current driver and see if it still fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just running ./scripts/download_driver_for_all_platforms.sh -f and rebuilding should be sufficient

Copy link
Contributor

Choose a reason for hiding this comment

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

it was failing on main for me too but you figured it out. downloaded the current driver and it's all good. apologies for that. i'll make running that command part of my workflow.

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.

3 participants