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

[Question] Unit tests ? #3

Open
RobbWatershed opened this issue Sep 13, 2021 · 10 comments
Open

[Question] Unit tests ? #3

RobbWatershed opened this issue Sep 13, 2021 · 10 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@RobbWatershed
Copy link
Contributor

I'd like to contribute a few additions but, as there's no documentation and no design documents, I'm kinda afraid to break things without realizing it.

I guess you're testing new changes through your app, which is totally fine as far as you're concerned, but imo not enough for a library aimed at being reusable outside of that app.

=> Is there a way we could set up unit tests to detect any regression and somehow show how the API works ?

I'm not sure how to start, but I would definitely help enrich the first tests if you got a few of them running...

@tom5079
Copy link
Owner

tom5079 commented Sep 13, 2021

I agree with you but I am also not sure how to start mainly because of the "maintaining URI" part.
I think one option is to modify the sample app to test things out, but I have no idea how to implement it as unit tests at the moment.

You can just submit PR and modify codes however you want, seems like I'm the only one who is using this library seriously 😂

@tom5079 tom5079 self-assigned this Sep 13, 2021
@tom5079 tom5079 added documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Sep 13, 2021
@tom5079 tom5079 pinned this issue Sep 13, 2021
@tom5079
Copy link
Owner

tom5079 commented Sep 13, 2021

By the way, even the official DocumentFile library is not properly tested 🤷

this is all they're doing

@tom5079
Copy link
Owner

tom5079 commented Sep 13, 2021

OK. I think I found a way

The combination of MANAGE_EXTERNAL_STORAGE flag and android emulator github action should do the trick 🎉

@RobbWatershed
Copy link
Contributor Author

RobbWatershed commented Sep 13, 2021

By the way, even the official DocumentFile library is not properly tested 🤷

this is all they're doing

...and we know what it lead to 😆

Seriously, Google have done many outstanding things, but goddamn SAF is definitely not one of them 🤦

@RobbWatershed
Copy link
Contributor Author

You can just submit PR and modify codes however you want, seems like I'm the only one who is using this library seriously 😂

Well, I'm gonna do that PR anyway because the piece of code you got from avluis doesn't work on Android 11 (we just found out about that last week)

However, I still haven't abandoned the idea of externalizing file I/O to a library such as that one. It makes sense to join our forces to simplify that part of Android.

Please let me know when you have the first unit test running, I'll take some time to increase coverage 😉

@tom5079
Copy link
Owner

tom5079 commented Sep 14, 2021

okay, I think I found a way
MANAGE_EXTERNAL_STORAGE doesn't work for SAF 💩 so we have to resort to use UiAutomation API to manually automate the permission granting process.

@tom5079
Copy link
Owner

tom5079 commented Oct 3, 2021

@RobbWatershed

I pushed the code to dev branch that grants permission automatically.
Currently, it's quite limited (only supports SDK 21 and no CI support yet) but I'll figure them out shortly.

You can run the tests locally by running tests in sample/androidTest/ExampleInstrumentedTest.kt (yes, I should change the name too)

@RobbWatershed
Copy link
Contributor Author

As far as i'm concerned, API21 is fine. I'll take a look and get back to you soon.

@RobbWatershed
Copy link
Contributor Author

@tom5079 Sorry for the delay, I've been focused on my roadmap for quite a while.

Just tested your new unit test successfuly. What you've done to enable permissions is devilish... but hey, it works, so no complaints here ! 😉

I'll post some newer tests soon. Thanks for the hard work !

@tom5079
Copy link
Owner

tom5079 commented Dec 4, 2021

No problem, and thanks for your interest!
Tell me whatever needs to be done with this library 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants