forked from mmcc007/screenshots
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
c771087
commit 8ea5f7b
Showing
31 changed files
with
380 additions
and
397 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,8 @@ | ||
library screenshots; | ||
|
||
export 'src/run.dart' show screenshots; | ||
export 'src/config.dart'; | ||
export 'src/base/process_common.dart'; | ||
export 'src/capture_screen.dart'; | ||
export 'src/config.dart'; | ||
export 'src/globals.dart' show DeviceType, kConfigFileName; | ||
export 'src/utils.dart' show isAdbPath, isEmulatorPath; | ||
export 'src/base/process_common.dart'; | ||
export 'src/image_magick.dart' show isImageMagicInstalled; | ||
export 'src/run.dart' show screenshots; | ||
export 'src/utils.dart' show isAdbPath, isEmulatorPath; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @DrBu7cher !
I know this is not an official package/fork or anything like that. But it seems that is the only thing compatible with an up-to-date flutter project.
I am trying to migrate my old code from test_driver to flutter_test and use your package to generate screenshots.
My problem is that you have changed the signature of the
screenshot(driver, config, name)
method toscreenshot(tester,binding,integrationTestChannel,platformDispatcher,name)
. I have no clue what are the integrationTestChannel and platformDispatcher.I am unable to find a test or example using this functionality. (or another of your repos using this package)
Could you send me or update the README.md file with a simple example?
Thank you!!!
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, are you asking for putting this back into screenshots?
I'm using it like (pseudo-code)
e.g.
integration_test/int_test.dart
and then I run
screenshots -p
(For using with patrol)8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DrBu7cher thank you for the code, that is great.
I am getting a nasty error though. Patrol is expecting the test file to be in
integration_test/app_test.dart
(I think)Currently screenshots library uses the config file
tests
array for tests.The command to run patrol seems to be missing the parameter
--target
+testPath
to use the same path as screenshots. HEREGoing to move my tests there and check if it works.
Thank you again!
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you using patrol v2?
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what you are saying.
Still: If you run
patrol test
it usesintegration_test/test_bundle.dart
and collects all tests in the directory into one executable to run the tests with. So you actually would not need to use the--target
flag.If you only want one/explicit tests, I see what you mean.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
864ed81
Can you test if it works as expected please?
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying.... which version are you using?
I'll test the changes as soon as I can manage this working with the default patrol config.
Thanks
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
patrol: ^2.2.0
patrol_cli v2.1.2
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read through leancodepl/patrol#1341 to get v2 to work
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't use
group
in your test (usepatrolTest
only) and yourmain
in the test has to be synchronous - that is where I fell flat on my face when migrating.8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just ran my pipeline and at least the change did not break anything. But I don't know, theoretically I should be able to just run the Patrol test without specifying the target - and that should run all the tests in the
integration_test
folder.8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I will add this later, for me it is not a use case right now.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could not try much more, but I am stuck. It takes a long time to run, and when it finishes, it has not generated any screenshots.
I don't think it really executes the test file at all. Maybe I did screw up setting up Patrol. Tomorrow I'll try again with a smaller project and/or use flutter_test without Patrol, as I really do not need Patrol for my tests at this moment. It is really great, but it also raises the complexity significantly.
Thank you again @DrBu7cher
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey man,
sorry to hear that.
Theoretically (I did not test it yet) you can replace the patrol-specific parts and use
(See here) instead.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pseudo, written on my phone:
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi man!
Thank you for all the help. It has started working, but I am having project issues in both Android and iOS.
Just to share, in Android, because I use flutter_stripe package, I need to use
FlutterFragmentActivity
instead ofFlutterActivity
. And there is a bug reported thatflutter_test
cannot generate screenshots using the fragment activity class. So I am going to make a script to switch that class back and forth when running the tests. (and not have screenshots for stripe payments integration, not a big deal)In iOS, I have this issue only when using simulators... So I cannot generate iOS screenshots. Not sure what is my best alternative here to unblock myself.
Anyway, thank you again!
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have fixed the iOS problem 🎉
Now I get the following error running screenshots:
Any idea why is this?
Btw, can you share your screenshots.yaml file?
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I solved that one. It was due to the list of devices contained the iPhone 14 Pro Max, but the resources were missing.
Using iPhone 11 Pro Max works fine.
I am trying to capture screenshots for all the app, browsing across the app using taps. In ios works fine, I get all screenshots (small bug though: in the screenshots name, it replaces paces with %20... so screenshot with name like 'Login Screen' becomes:
`Ok, I solved that one. It was due to the list of devices contained the iPhone 14 Pro Max, but the resources were missing.
Using iPhone 11 Pro Max works fine.
I am trying to capture screenshots for all the app, browsing across the app using taps. In ios works fine, I get all screenshots (small bug though: in the screenshots name, it replaces paces with %20... so screenshot with name like 'Login - Screen' becomes:
iPhone 11 Pro Max-Portrait-Login%20-%20Screen.png
)But I found a big blocker in Android.
As I take the second screenshot, I get this error:
I guess this might be due to it tries to execute
convertFlutterSurfaceToImage
several times?Have you been able to generate several screenshots in Android?
Am I doing something wrong?
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey man,
happy to hear that at least the basic setup is working.
The
%20
is because the device is sending the screenshots pixels as a web request to your pc. I can check if it would be easy to add an urldecode step for the screenshots path.For the other problem: Yes, I have been able to take multiple screenshots. The
screenshots
package is reverting the call every time for android: https://github.com/DrBu7cher/screenshots/blob/master/lib/src/capture_screen.dart#L33The first line in the error output you send comes from here: https://github.com/DrBu7cher/screenshots/blob/master/lib/src/capture_screen.dart#L89
The rest is an uncaught exception. Which I find weird, because the surface is most probably an image already. Did you set a timeout on
return await screenshot([...]
and/or is it very small?Because then it would not pump a frame, which it has to. I can push a fix, if this is the cause of your issue, but please check for me first.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Jonas!
Yes, there is an await when taking the screenshot, and I had a timeout of 1 second in my tests.
I am unsure what the problem is... going to continue making changes and see if that works.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! I've fixed it following this: flutter/flutter#92381 (comment)
Going to send you a PR, ok?
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in general, you don’t have to ask for permission to open a PR. :)
So of course, open one where I can see exactly what fixed it for you.
I find it weird that it is working for me, but I did not test it with the native implementation yet.
Please only make sure you are using the latest flutter version (stable if you want) to make sure the fix is still necessary.
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3 PRs are running: Flutter 3.13.2 • channel stable
8ea5f7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contributions!
I ran my pipeline for both Android and iOS so it is not broken with Patrol either. :)