-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Enable plugin_test_linux #120912
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
Enable plugin_test_linux #120912
Conversation
Enables the new end-to-end Linux desktop plugin tests.
|
@godofredoc do you have any pointers on the error here? Do I just need to add the |
|
@godofredoc Ping on the question above; is a config change all that's needed to make |
Yes, let's add it as a dependency to the .ci.yaml file. It will be added automatically to the path. If it doesn't work please ping me a build link to help triage. |
Speculative fix for the failure discussed in #120912
|
@godofredoc It's still failing with the same error, unfortunately: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20plugin_test_linux/2/overview Any idea how we can make this work? I haven't seen a failure like this outside of LUCI. |
|
@keyonghan FYI |
|
Will take a look after a P0 issue. |
|
I can verify that the @stuartmorgan Do you know how the curl command is executed from the test? By searching around, the |
|
It's run by CMake as part of the build; see https://github.com/flutter/flutter/blob/master/packages/flutter_tools/templates/plugin/linux.tmpl/CMakeLists.txt.tmpl#L65-L69 Looking around a bit more online now that I know it's only happening with CMake, I found some discussion suggesting that it may have to do with how the installed version of CMake was built and what |
|
We are using the Ran an led with latest version from 3pp, it passed the curl error, though the test still fails. I will land some quick fixes for the package change. |
|
That's just a path mistake in the test code; I can update this PR to fix that tomorrow. Thanks for tracking down the curl problem! |
| - bin/** | ||
| - .ci.yaml | ||
|
|
||
| - name: Linux plugin_test_linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Led run succeeds with the new cmake version: https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/d813b8f1ffe0863aff0697eaff6a6e86b51e2a8af06502fcb345af4e7e523afb/+/build.proto?server=chromium-swarm.appspot.com
Updating the cmake version to version:[email protected] should pass the CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! That has to be a separate PR that lands and then propagates I assume, like other ci.yaml changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presubmit CI will pick up the changes right away, so changes in this same PR will work.
Please allow https://flutter-review.googlesource.com/c/recipes/+/40060 to land before pushing the change, as the infra config complains about the special character @ in the version: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20ci_yaml%20flutter%20roller/2119/overview.
After the above CL lands, please use version build_id:8787856497187628321 instead of version:[email protected].
(successful led run with version build_id:8787856497187628321: :https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/9a8f995890c8aa1dd97b86de8a5f3a2018dcce8050fcc2f2143d6b6569aa0e5c/+/build.proto?server=chromium-swarm.appspot.com)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presubmit CI will pick up the changes right away, so changes in this same PR will work.
Ah, nice. Is there a list somewhere of what does and doesn't work in-PR? I know that timeout changes don't take effect right away, for instance, having been bitten by that before, but I'm not clear on what things do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All properties should be picked up right away. Updated flutter/cocoon#2519 to make it clear.
|
|
keyonghan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks for all the help with resolving the blocker! |
Enable plugin_test_linux
Enables the new end-to-end Linux desktop plugin tests.
Pre-launch Checklist
///).