Skip to content

Add a trybot for flutter customer tests #51042

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

Open
srawlins opened this issue Jan 17, 2023 · 15 comments
Open

Add a trybot for flutter customer tests #51042

srawlins opened this issue Jan 17, 2023 · 15 comments
Assignees
Labels
area-build Use area-build for SDK build issues. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

I have landed several changes into dart-lang/sdk, which have each ultimately caused some bot somewhere to fail, as each change causes one or most customer tests to fail in the flutter/tests repo. There is no trybot for these "flutter customer tests."

This current feedback loop is very expensive for me and for the flutter engine rollers:

  1. it takes several days for me to find out the change causes new failures in flutter/tests
  2. it breaks some flutter auto-roller or other
  3. breaking the auto-roller requires humans to identify the culprit, and revert my change
  4. I must fix each customer's codebase to be compatible with my change
  5. I must bump the commit SHA of each customer's codebase in flutter/tests
  6. I land again, without any customer test trybots, cycling back to the top of this feedback loop.

CC @Hixie @jason-simmons @iskakaushik to perhaps fill in details I got wrong above

@srawlins srawlins added type-enhancement A request for a change that isn't a bug area-build Use area-build for SDK build issues. labels Jan 17, 2023
@srawlins
Copy link
Member Author

@Hixie
Copy link
Contributor

Hixie commented Jan 18, 2023

The flutter/tests tests are designed to be runnable from any repo, so it should be possible to run them upstream. I forget if we have an environment variable to force a local engine to be used (I presume you'd have to build a local engine and use that to run the tests to test Dart changes), but if we don't we should be able to add one. cc @christopherfujino

@whesse
Copy link
Contributor

whesse commented Jan 18, 2023

Regarding local engine: The engine_v2 recipes run tests with a custom engine by passing a custom FLUTTER_STORAGE_BASE_URL and using flutter tools, not using the local-engine flag.

These tests are run through the adhoc_validation recipe module, and not through the standard test script 'dev/bots/test.dart'.

So to add this (to monorepo testing), we will create a new validation_tester recipe that can be triggered from the engine_v2 api, that spawns testers in new builds after the engine build is complete. We already have an engine_v2/tester that runs dev/bots/test.dart, but this should be a different recipe to run adhoc_validation steps instead.

We will have to make sure that the adhoc_validation steps run correctly on a flutter/flutter checkout with a custom artifact download URL - we have a bug filed because custom artifact download URLs break some tests that run flutter tools with a --fatal-warnings flag, but are working on fixing this.

This change would need to be made in the flutter recipes anyway, as part of the new engine_v2 Build/Archive/Test separation they are moving their builds to, so we will just do it earlier.

CC @godofredoc

The timeline for implementing this will be weeks, not days.

@srawlins
Copy link
Member Author

Thanks much for all of the context, @whesse !!!

The timeline for implementing this will be weeks, not days.

Very understandable, haha. Having this implemented in weeks would be a huge win for the analyzer team at least, and probably more. Thanks!

@srawlins
Copy link
Member Author

Is it possible this might be prioritized for Q3? Thanks!

@a-siva
Copy link
Contributor

a-siva commented Aug 25, 2023

Is it possible this might be prioritized for Q3? Thanks!

@godofredoc any possibility work on this could be bumped, when we run into issues with this codebase it is problematic for us to see if we have a fix or not without having the ability to run the changes locally

@godofredoc
Copy link
Contributor

I can take a look next week to see if we can move customer testing to dev/bots/test.dart as it is only 4 commands in bash https://cs.opensource.google/flutter/recipes/+/main:recipe_modules/adhoc_validation/resources/customer_testing.sh

@kallentu
Copy link
Member

I ran into this a couple days ago where my change in the Dart SDK broke a Flutter customer. And there were two layers of errors, so it took a day or two to figure out it was my change and then my first forward fix fixed the first layer of errors, but there were more that I was unaware about until another day or two after. Then when I was creating a fix, the turnaround time for testing it would be up to half a day.

It's not a great experience to be mostly certain about the effects of your changes and need to test by waiting for multiple versions and rolls to go through.

Would love to see additional testing on the Dart pipeline 👍

@whesse whesse added the P2 A bug or feature request we're likely to work on label Oct 24, 2023
@whesse
Copy link
Contributor

whesse commented Oct 24, 2023

This is now on my list of priority tasks to do, it is clearly an important request.

@srawlins
Copy link
Member Author

Thanks much Bill, really super appreciated!

@srawlins
Copy link
Member Author

srawlins commented Nov 9, 2023

If it's helpful to have some examples to play with, I'd like to run these three CRs against the flutter customer tests in try-bots:

auto-submit bot pushed a commit to flutter/flutter that referenced this issue Nov 28, 2023
This will allow Dart Team to run customer tests as part of monorepo and will be a step forward to remove ad_hoc tests.

Bug: dart-lang/sdk#51042
Bug: #115476
@godofredoc
Copy link
Contributor

Customer_testing is now a shard test and it is ready to be added to the monorepo configs. @whesse FYI

copybara-service bot pushed a commit that referenced this issue Dec 12, 2023
Bug: #51042
Change-Id: Id2aeec14c23d105f7d61516d5949b216dccda2c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341220
Reviewed-by: Alexander Thomas <[email protected]>
Commit-Queue: William Hesse <[email protected]>
@whesse
Copy link
Contributor

whesse commented Dec 12, 2023

Customer_testing has been added to monorepo, and is now running on CI. It is available as a try job for Dart SDK CLs by adding the flutter-linux-try builder with the "Choose Tryjobs" link.

@srawlins
Copy link
Member Author

Thanks so much, Bill!!! I'll be using this a lot this week. Much appreciated!

@godofredoc
Copy link
Contributor

Thank you @whesse

caseycrogers pushed a commit to caseycrogers/flutter that referenced this issue Dec 29, 2023
This will allow Dart Team to run customer tests as part of monorepo and will be a step forward to remove ad_hoc tests.

Bug: dart-lang/sdk#51042
Bug: flutter#115476
whesse added a commit to whesse/flutter that referenced this issue Jan 5, 2024
The migration of customer tests to sharded tests adds a step
that checks out the current tip-of-tree of the framework repo,
removing local changes. This does not work with monorepo
testing, which modifies engine.version, and does not work with
local testing of a branch.

The sharded tests should already be running with the correct
checkout of the framework repo. If the REVISION environment
variable is set, the framework checkout will still be
reset to check out that revision.

These commands were migrated from the existing shell script
to the sharded tester in
flutter#138659

Bug: dart-lang/sdk#51042
auto-submit bot pushed a commit to flutter/flutter that referenced this issue Jan 12, 2024
The migration of customer tests to sharded tests adds a step that checks out the current tip-of-tree of the framework repo, removing local changes. This does not work with monorepo testing, which modifies engine.version, and does not work with local testing of a branch.

The sharded tests should already be running with the correct checkout of the framework repo. If the REVISION environment variable is set, the framework checkout will still be reset to check out that revision.

These commands were migrated from the existing shell script to the sharded tester in
#138659

Bug: dart-lang/sdk#51042
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-build Use area-build for SDK build issues. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants