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

import_js_library is now using getAssetUrl to resolve no_sleep.js. #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

diegotori
Copy link
Collaborator

Addresses issue #19 from flutter/flutter#121417 (comment).

@ditman
Copy link

ditman commented Feb 16, 2024

This is how we currently do integration testing in flutter/packages:

@diegotori
Copy link
Collaborator Author

This is how we currently do integration testing in flutter/packages:

I've tried that, but if there's something deep in the JS that doesn't bubble up to the test, then the test may not give an exact reason why it failed.

That and I'm unable to debug this by stepping through the code, let alone keeping Chrome open long enough for me to debug the JS that it calls on that platform.

@ditman
Copy link

ditman commented Feb 16, 2024

@diegotori you can keep chrome open for as long as you need by running your test as if it were the main of your app:

flutter run -d web-server --web-port 7357 integration_test/name_of_your_test.dart

That's how I usually debug those tests; you can set breakpoints, refresh the browser, restart the compiler (if you keep the web-port constant you don't have to close the browser)... whatever you need.

(I don't use Android Studio, so I don't know if it can be integrated nicely with the IDE)

if there's something deep in the JS that doesn't bubble up to the test, then the test may not give an exact reason why it failed.

In this case, you might not be able to test that the browser stays alive for an extended period of time, but you can surely assert in your test that certain elements in the page with the correct routes, or that some JS variable was added to the window by the JS that your test includes... In this case, I'd assert at least the path to the no_sleep.js file in the src attribute of the script tag that the plugin adds (maybe?)

@diegotori
Copy link
Collaborator Author

diegotori commented Feb 19, 2024

@diegotori you can keep chrome open for as long as you need by running your test as if it were the main of your app:

flutter run -d web-server --web-port 7357 integration_test/name_of_your_test.dart

That's how I usually debug those tests; you can set breakpoints, refresh the browser, restart the compiler (if you keep the web-port constant you don't have to close the browser)... whatever you need.

(I don't use Android Studio, so I don't know if it can be integrated nicely with the IDE)

@ditman just curious as to how one can step through breakpoints using this method, since I tried it via command line, and it's not even opening Flutter DevTools for me. It only opened it when I changed the device to chrome instead of web-server.

Not to mention, when I open the localhost page for that port, it goes through the tests, but because there's no Flutter DevTools URL, there's no way for me to debug it there.

@ditman
Copy link

ditman commented Feb 21, 2024

how one can step through breakpoints using this method [...] because there's no Flutter DevTools URL, there's no way for me to debug it there.

You're right, when I debug this way I normally use the vanilla Chrome DevTools and its JS breakpoints, and not the Dart DevTools.

(Sourcemaps are a little bit random, some times they work, others they don't)

@diegotori
Copy link
Collaborator Author

how one can step through breakpoints using this method [...] because there's no Flutter DevTools URL, there's no way for me to debug it there.

You're right, when I debug this way I normally use the vanilla Chrome DevTools and its JS breakpoints, and not the Dart DevTools.

(Sourcemaps are a little bit random, some times they work, others they don't)

I see. So I should be stepping over the JS in the browser to confirm that Flutter called out it?

Then that makes sense. I'll try that approach.

Just wish that it was trivial to step through Flutter Web based integration tests. Right now, this is not an issue for iOS and Android, but apparently the Flutter Driver doesn't support Chrome as a device at the moment.

@diegotori
Copy link
Collaborator Author

@ditman After running it through Chrome DevTools, it looks like it's actually resolving the no_sleep.js script on the platform side of things, even hitting all of the functions in the integration test.

What's weird though is that when running flutter drive -d web-server --web-port 7357 --browser-name chrome --driver test_driver/integration_test.dart --target integration_test/wakelock_plus_test.dart in the example project, it shows up as a failure, even though in Chrome DevTools, it prints that all the tests passed without issue.

I'm just wondering the best way to automate this as part of the workflow so that this gets covered going forward, without generating any false positive/negatives?

@ditman
Copy link

ditman commented Feb 21, 2024

@diegotori can you share the code of your wakelock_plus_test.dart? The main issues I've found when doing these types of tests in the browser vs the driver is that there's subtle timing issues/race conditions that make your test fail (like: you're expecting something now that should be expectLater, forgetting some await somewhere, or similar). It's hard to debug without the test code though!

@diegotori
Copy link
Collaborator Author

@ditman Here you go: https://github.com/fluttercommunity/wakelock_plus/blob/main/wakelock_plus/example/integration_test/wakelock_plus_test.dart

All it really does is that it enables, disables, and toggles the wakelock implementation on the platform.

@ditman
Copy link

ditman commented Feb 21, 2024

@diegotori My suspicion is that the await is not awaiting enough for the wakelock to being enabled (or the other way around). I don't know the specific implementation details, so I don't have a super strong opinion about this. The test looks good!

Something I've done in the past is create tests only for the _web package of the plugin, which allow me to access more detailed browser APIs (because I know those tests will run on the browser only!)

@diegotori
Copy link
Collaborator Author

@ditman So the original author of the wakelock plugin initially wrote a test for that, but wasn't able to get it working: https://github.com/fluttercommunity/wakelock_plus/blob/main/wakelock_plus/test/wakelock_plus_web_plugin_test.dart.

Even when running it using flutter test, it says that no tests were found.

I'm still stumped as to how to properly get that test working.

@ditman
Copy link

ditman commented Feb 22, 2024

@diegotori I created this PR with some ideas, feel free to take any of them or leave them:

Even when running it using flutter test, it says that no tests were found.

flutter test --platform chrome should find the tests that are annotated with TestOn('chrome') or similar.

I couldn't get the automated test to work either, it fails to load the JS asset (wrong URL possibly?), but I don't have time to look more into this, sorry!

@ditman
Copy link

ditman commented Feb 22, 2024

Another note is that maybe the JS asset needs to be inside lib so it can be used from other packages too?

See: https://stackoverflow.com/questions/54740732/how-to-add-assets-in-flutter-package-plugin-development

I'm not 100% positive what's failing, sorry for not being more helpful!

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.

2 participants