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.

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

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

Open
5 tasks
kenzieschmoll opened this issue Jun 14, 2024 · 10 comments
Open
5 tasks

Comments

@kenzieschmoll
Copy link
Member

kenzieschmoll commented Jun 14, 2024

  • 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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

ericwindmill commented Jun 26, 2024

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
Copy link
Member Author

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
Copy link
Contributor

kevmoo commented Jun 26, 2024

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

@ericwindmill
Copy link
Contributor

ericwindmill commented Jun 26, 2024

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
Copy link
Member

ditman commented Jul 2, 2024

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:

It ain't much but...

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

No branches or pull requests

5 participants