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.

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

Fix broken integration tests #8299

Merged
merged 18 commits into from
Sep 6, 2024
Merged

Fix broken integration tests #8299

merged 18 commits into from
Sep 6, 2024

Conversation

kenzieschmoll
Copy link
Member

@kenzieschmoll kenzieschmoll commented Sep 4, 2024 •
edited
Loading

This PR adds flags to the flutter drive command for DevTools integration tests to support the latest version of chrome and chromedriver 128.

        // --disable-gpu speeds up tests that use ChromeDriver when run on
        // GitHub Actions. See https://github.com/flutter/devtools/issues/8301.
        '--web-browser-flag=--disable-gpu',
        // Flags to avoid breakage with chromedriver 128. See
        // https://github.com/flutter/devtools/issues/8301.
        '--web-browser-flag=--headless=old',
        '--web-browser-flag=--disable-search-engine-choice-screen',

Chrome has a new headless mode, so we needed to pass --headless=old to get the integration tests to function as usual. Following a suggestion from SeleniumHQ/selenium#14453 and sanger/sequencescape#4316, we also needed to add the --disable-search-engine-choice-screen flag.

At some point if flutter driver supports the new headless mode, we should be able to remove the flags here. Fixes #8301.

@kenzieschmoll kenzieschmoll mentioned this pull request Sep 4, 2024
Closed
@kenzieschmoll kenzieschmoll changed the title Test branch for bots Fix broken integration tests Sep 5, 2024
@kenzieschmoll kenzieschmoll requested a review from DanTup September 5, 2024 17:06
@DanTup
Copy link
Contributor

DanTup commented Sep 5, 2024

The changes seem reasonable to me (though I don't know if the intention is to land it with the additional debug logging), though seems they're still failing:

FlutterDriveProcess - Application finished.
Unhandled exception:
Exception: Unhandled exception:
DriverError: Driver extension has not been initialized correctly.
If the test uses a custom VM service extension, make sure it conforms to the protocol used by package:integration_test and package:flutter_driver.
If the test uses VM service extensions provided by the Flutter SDK, then this error is likely caused by a bug in Flutter. Please report it by filing a bug on GitHub:
  https://github.com/flutter/flutter/issues/new?template=2_bug.yml
Original error: ScriptTimeoutException (500): script timeout
  (Session info: chrome=128.0.6613.84)
Original stack trace:
#0      parseW3cResponse (package:webdriver/src/handler/w3c/utils.dart:78:9)
#1      W3cCoreHandler.parseExecuteResponse (package:webdriver/src/handler/w3c/core.dart:59:19)

@kenzieschmoll kenzieschmoll marked this pull request as ready for review September 6, 2024 00:17
@kenzieschmoll kenzieschmoll requested review from bkonyi and a team as code owners September 6, 2024 00:17
@kenzieschmoll
Copy link
Member Author

@DanTup this is ready for review. Removing the --headless flag did not work, but I was able to get headless mode working with a few extra flags. The debug logs are intentional and will only be printed when the flag for debug logging is true.

DanTup
DanTup approved these changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@DanTup DanTup left a comment

Choose a reason for hiding this comment

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

LGTM!

packages/devtools_shared/lib/src/test/integration_test_runner.dart Outdated
Comment on lines 56 to 57
'--web-browser-flag=--headless=old',
'--web-browser-flag=--disable-search-engine-choice-screen',
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's how you can pass these through 😃

@bkonyi
Copy link
Contributor

bkonyi commented Sep 6, 2024

FYI @derekxu16, this looks very similar to the issues we were having with the DWDS testing. Do we need to pass any of these other flags?

@derekxu16
Copy link
Contributor

FYI @derekxu16, this looks very similar to the issues we were having with the DWDS testing. Do we need to pass any of these other flags?

I think the DWDS checks are working fine now. If they start failing again then we should try passing these other flags.

@kenzieschmoll kenzieschmoll mentioned this pull request Sep 6, 2024
Open
@kenzieschmoll kenzieschmoll merged commit 3127636 into flutter:master Sep 6, 2024
23 checks passed
@kenzieschmoll kenzieschmoll deleted the test-branch branch September 6, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkonyi bkonyi bkonyi approved these changes

@DanTup DanTup DanTup approved these changes

Assignees
No one assigned
Labels
release-notes-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DevTools integration tests are broken on the CI
4 participants
@kenzieschmoll @DanTup @bkonyi @derekxu16