Skip to content

Fix failing tests for RawAutocomplete #1190

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

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

victorsanni
Copy link
Contributor

The fixes to RawAutocomplete's options width in the flutter framework (flutter/flutter#143249) introduces a frame delay, so this test requires an extra pump to account for that.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Dec 20, 2024

Welcome, @victorsanni! Are you here because you chose option 2 of the broken-test policy of the Flutter Customer Test Registry?

  1. Go through the breaking change process, as documented here: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes If you're going to do this, you will need to contact the relevant people responsible for the breaking test(s) (see the relevant .test files), help them fix their code, and update this repository to use the new version of their tests, in addition to the steps described on the wiki. You will also need to land your change in two parts, so that people have time to migrate (a "soft-breaking" change).

I'd like to better understand the rationale for this breaking change. A frame delay doesn't sound ideal; can your bugfixes be done without it? If not, I'd like to see an explanation.

@victorsanni
Copy link
Contributor Author

victorsanni commented Dec 20, 2024

Hi @chrisbobbe! Indeed I am here for that reason.

The associated Flutter framework PR fixes the issue where the RawAutocomplete widget goes out of the screen from the right side. But the tradeoff was that unfortunately, the layout of the options view doesn't update on the same frame that the field layout changes.

@gnprice suggested some ideas on how to eliminate the one frame delay, which I implemented in the PR to make the options view relayout when the constraints given to the field change. However, there are still cases where the frame delay remains, which (I think) is causing the failing test.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Dec 23, 2024

@gnprice suggested some ideas on how to eliminate the one frame delay, which I implemented in the PR to make the options view relayout when the constraints given to the field change. However, there are still cases where the frame delay remains, which (I think) is causing the failing test.

It sounds like you found that Greg's ideas had gaps in them 🙂 because they didn't work for some cases. Is there a description of those gaps that you can point Greg to? Then perhaps he'll suggest some new ideas, or else agree and corroborate your claim that the new lag is necessary.

@gnprice gnprice force-pushed the autocomplete-extra-pump branch from 495d113 to 001a853 Compare January 15, 2025 17:23
…mplete

The fixes to RawAutocomplete's options width in the Flutter framework:
  flutter/flutter#143249

introduces a one-frame delay, so this test will need an extra pump to
account for that.
@gnprice gnprice force-pushed the autocomplete-extra-pump branch from 001a853 to 0bfa79d Compare January 15, 2025 17:36
@gnprice gnprice merged commit 0bfa79d into zulip:main Jan 15, 2025
@gnprice
Copy link
Member

gnprice commented Jan 15, 2025

Thanks @victorsanni for sending this fix! Sorry we didn't follow up again on this sooner. Merged now (and adapted the commit message to our style).

I'm still curious how things turned out in that upstream PR — I recall at one point last month you believed the delay was resolved after all, due to new ideas from LongCat — but I'll catch up there.

gnprice added a commit to gnprice/flutter_customer_testing that referenced this pull request Jan 15, 2025
This has the update to unblock autocomplete changes:
  zulip/zulip-flutter#1190
  flutter/flutter#143249

and with that, tests pass again on Flutter main.
auto-submit bot pushed a commit to flutter/tests that referenced this pull request Jan 21, 2025
This has the update to unblock autocomplete changes:
  zulip/zulip-flutter#1190
  flutter/flutter#143249

and with that, tests pass again on Flutter main.

Also install libsqlite3-dev on Linux, a package which is no longer
included in GitHub's new version of the "ubuntu-latest" images.  The
lack of it causes nondeterministic failures when the runner happens
to use the new image:
  #441 (comment)

The actual shared library file libsqlite3.so.0.8.6 -- in the package
libsqlite3-0 -- is still installed by default anyway.  This package
adds the symlink at "libsqlite3.so", which is the filename some
libraries apparently look for.
auto-submit bot added a commit to flutter/tests that referenced this pull request Jan 22, 2025
Reverts: #441
Initiated by: matanlurey
Reason for reverting: sudo apt-get install does not work on LUCI
Original PR Author: gnprice

Reviewed By: {Piinks}

This change reverts the following previous change:
This has the update to unblock autocomplete changes:
  zulip/zulip-flutter#1190
  flutter/flutter#143249

and with that, tests pass again on Flutter main.

Also install libsqlite3-dev on Linux, a package which is no longer
included in GitHub's new version of the "ubuntu-latest" images.  The
lack of it causes nondeterministic failures when the runner happens
to use the new image:
  #441 (comment)

The actual shared library file libsqlite3.so.0.8.6 -- in the package
libsqlite3-0 -- is still installed by default anyway.  This package
adds the symlink at "libsqlite3.so", which is the filename some
libraries apparently look for.
victoreronmosele pushed a commit to victoreronmosele/tests that referenced this pull request Feb 5, 2025
This has the update to unblock autocomplete changes:
  zulip/zulip-flutter#1190
  flutter/flutter#143249

and with that, tests pass again on Flutter main.

Also install libsqlite3-dev on Linux, a package which is no longer
included in GitHub's new version of the "ubuntu-latest" images.  The
lack of it causes nondeterministic failures when the runner happens
to use the new image:
  flutter#441 (comment)

The actual shared library file libsqlite3.so.0.8.6 -- in the package
libsqlite3-0 -- is still installed by default anyway.  This package
adds the symlink at "libsqlite3.so", which is the filename some
libraries apparently look for.
victoreronmosele pushed a commit to victoreronmosele/tests that referenced this pull request Feb 5, 2025
Reverts: flutter#441
Initiated by: matanlurey
Reason for reverting: sudo apt-get install does not work on LUCI
Original PR Author: gnprice

Reviewed By: {Piinks}

This change reverts the following previous change:
This has the update to unblock autocomplete changes:
  zulip/zulip-flutter#1190
  flutter/flutter#143249

and with that, tests pass again on Flutter main.

Also install libsqlite3-dev on Linux, a package which is no longer
included in GitHub's new version of the "ubuntu-latest" images.  The
lack of it causes nondeterministic failures when the runner happens
to use the new image:
  flutter#441 (comment)

The actual shared library file libsqlite3.so.0.8.6 -- in the package
libsqlite3-0 -- is still installed by default anyway.  This package
adds the symlink at "libsqlite3.so", which is the filename some
libraries apparently look for.
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