[testing_app] meta tracking issue for missing or inaccurate examples

kenzieschmoll commented 2 weeks ago

[ ] Does not follow guidance for where to place the test driver file. integration_test documentation and Flutter documentation suggest that the test driver should be under test_driver/integration_test.dart.
[ ] Readme does not provide documentation for how to run integration tests for the Web (e.g. adding -d [web-server | chrome] and starting chromedriver).

[ ] Running a simple integration test on chrome fails with opaque message

flutter drive \
--driver=integration_test/driver.dart \
--target=integration_test/app_test.dart \
-d chrome
...
Rejecting promise with error: TypeError: Cannot read properties of undefined (reading 'group')

[ ] Missing examples of benchmark testing
[ ] Open question: I could not tell from looking at the code. Are integration tests ran on the Flutter CI? Are they ran on web on the CI?

domesticmouse commented 2 weeks ago

This sample was a summer of code sample. I'm happy for it either to be updated to meet specifications, or for it to be deleted as unmaintained.

ericwindmill commented 1 week ago

I would advocate for this sample being deleted. Testing is a topic better suited for documentation IMO. I'll delete delete it unless there are any strong opinions otherwise.

kenzieschmoll commented 1 week ago

Testing is a topic better suited for documentation IMO

While documentation is helpful as a starting point, it can become stale quickly and is not always as helpful as a real example in code. Especially for more complicated testing scenarios like integration testing and benchmark testing, a real example that is continuously tested against the latest Flutter & Flutter testing packages is essential.

ericwindmill commented 6 days ago

I don't think flutter/samples is the best place to collect Flutter code who's primary function is to test against new releases. I'm open to being wrong about this.

kenzieschmoll commented 6 days ago

Since flutter/gallery is now deprecated, I'm not sure what our plan is for providing up-to-date examples for end users. Based on the flutter/gallery README, it seems that flutter/samples is a resource we expect users to use: https://github.com/flutter/gallery/blob/main/README.md.

@mit do you have thoughts on this? What is our plan to provide users with up-to-date code examples that are compatible with the latest Flutter framework and Flutter package changes?

ericwindmill commented 6 days ago

I think I should've elaborated more.

This is the primary place to provide up-to-date code examples which are primarily used as a teaching tool. If the sample exists primarily to test against releases, it should be deprecated from this repository. This repository should be highly curated, and less is more. Offering a few high quality, complete, samples that cover happy paths is preferable to trying to cover every aspect of the SDK, imo.

Equally as importantly, we have a maintenance overload problem on the DevRel team. We're technically keeping the lights on with this samples, but we aren't spending time making sure the samples are still "good" samples. For example, I would guess that many have not been updated to use Dart 3 features. I am going to suggest deprecating a lot of the samples in this repo in Q3. (FWIW, finding a 'new approach' for DevRel samples that is 'more effective and maintainable' is one of my primary goals for the year.)

I'm open to being wrong about whether or not this particular sample should be deprecated. I'm also open to this repository being a place where code is primarily added to test against releases, but I don't think it should be both.

kenzieschmoll commented 6 days ago

This is the primary place to provide up-to-date code examples which are primarily used as a teaching tool.

It may be very difficult to provide "up-to-date" code examples if this repo is not regularly analyzed and tested against some continuous integration framework. For the area of testing in particular, I have run into this issue many times over the years which has resulted in a very painful developer experience to setup integration tests and benchmark tests for a Flutter Web app (Dart DevTools). TL;DR is that the documentation was conflicting, outdated, and sparse, and that the code examples were incomplete and incorrect. I have a detailed write-up on this that I recently shared with the Flutter web team, but we really need some documentation and code examples for testing that we can confidently put in users hands to guide them.

Whether that should be done in the samples/testing_app or not, I don't know, but I do know that having a single source of truth for testing examples that is accurate, guaranteed by CI to be up-to-date and passing, and documented well would really improve the developer experience here.

CC @yjbanov @eyebrowsoffire @kevmoo @ditman since we just discussed this topic WRT to Flutter Web.

...less is more. Offering a few high quality, complete, samples that cover happy paths is preferable to trying to cover every aspect of the SDK, imo.

Agreed. Though I will cast my vote for testing to be one of the critical aspects of the SDK that we provide high-quality and complete samples for.

I'm open to being wrong about whether or not this particular sample should be deprecated. I'm also open to this repository being a place where code is primarily added to test against releases, but I don't think it should be both.

If we were to keep the testing_app sample here and improve it to be the flagship example of how to write tests (all kinds) in flutter for any platform, then perhaps we could add this package to the flutter customer test registry so that all the testing_app tests would be run on the flutter/flutter CI? Although this wouldn't catch issues right away since some packages like package:integration_test and package:web_benchmarks are developed in the flutter/packages repo, so we'd only see errors once the deps were migrated in to flutter/flutter.

Getting into the weeds here a bit, but to bring it back to high level, we could find some way to run tests / analyze samples on the CI to guarantee a sample like one for testing is up to date.

kevmoo commented 6 days ago

Having weekly CI running against the latest stable release seems like a no-brainer, correct?

ericwindmill commented 6 days ago

It may be very difficult to provide "up-to-date" code examples if this repo is not regularly analyzed and tested against some continuous integration framework.

Having weekly CI running against the latest stable release seems like a no-brainer, correct?

I think our disagreement is actually just a miscommunication. I'm certainly not suggesting that we remove CI from this repo. I'm trying to clarify that if the primary purpose of a sample in this repo test an API against releases, we should consider deprecating it, because that's not what this repo is for. But yes, if it is in this repo, it should absolutely have tests and be test against stable often (and main and beta, for that matter.)

TL;DR is that the documentation was conflicting, outdated, and sparse, and that the code examples were incomplete and incorrect. I have a detailed write-up on this that I recently shared with the Flutter web team, but we really need some documentation and code examples for testing that we can confidently put in users hands to guide them.

I totally understand this concern, and agree that there's much documentation that can be improved about testing and otherwise. (One of the reasons I'd like to reduce our maintenance load on the DevRel team is so that DREs can contribute to the website more and not leave it all to 1.5 TWs.)

If we were to keep the testing_app sample here and improve it to be the flagship example of how to write tests (all kinds) in flutter for any platform, then perhaps we could add this package to the flutter customer test registry so that all the testing_app tests would be run on the flutter/flutter CI? Although this wouldn't catch issues right away since some packages like package:integration_test and package:web_benchmarks are developed in the flutter/packages repo, so we'd only see errors once the deps were migrated in to flutter/flutter.

I think having one flagship sample app about testing is a great idea and this is the right place for it. Having exactly one flagship app for every "major part" of the SDK (of which testing is a good example) is what I'm going to suggest we do when I have time to think holistically about updating all of our sample code.

So long story long, I guess I'm advocating for doing nothing right now.

ditman commented 15 hours ago

As a side-note, in flutter/packages, every user-facing package that has integration tests, are run against all supported platforms (by the plugin and by CI, of course); for example:

https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter/example/integration_test

It ain't much but...

flutter / samples

[testing_app] meta tracking issue for missing or inaccurate examples #2334