Piwigo / Piwigo-Android

Piwigo Native Android App
GNU General Public License v3.0
140 stars 43 forks source link

Add espresso test handling 2 accounts on the same gallery #224

Closed coolo closed 4 years ago

coolo commented 4 years ago

I setup https://tg1.kulow.org with 3 galleries. One public visible to everyone, one small for 'seessmall:seessmall' to see and one large for 'seeslarge:seeslarge' to see.

The espresso test adds both accounts

ramack commented 4 years ago

ah and what is needed to run the espresso tests? is it already executed in travis CI?

coolo commented 4 years ago

https://tilns.herokuapp.com/posts/ededca9597-android-espresso-waiting-for-viewsfragments is another approach to avoid sleeps, but it's quite intrusive into the production code.

No, it's not executed in travis CI (yet) - also it's failing.

You need to have an (english) device connected to android studio and execute connectedDebugAndroidTest gradle task. If you setup KVM, the emulator is not as bad.

I will first fix the test and then hook it into travis CI.

ramack commented 4 years ago

another approach to avoid sleeps, but it's quite intrusive into the production code. Is it really worth that? We don't want to win a software-engineering-award with the test, so let's try to be pragmatic.

connectedDebugAndroidTest That might be worth documenting in the wiki.

BTW: thanks for working on testing! When reading the test code I really get a bad conscience of not having done it earlier!

coolo commented 4 years ago

Is it really worth that? We don't want to win a software-engineering-award with the test, so let's try to be pragmatic.

Hence the sleep ;)

coolo commented 4 years ago

Running it on travis-ci is possible, but seems to trigger a server bug :(

It returns the wrong list of galleries for seessmall (only Public). I can make it run once if I purge sessions and user cache on the server, but the run afterwards will fail again :(

coolo commented 4 years ago

See https://travis-ci.com/coolo/Piwigo-Android/jobs/273194135#L4772 for the server reply

Philio commented 4 years ago

You will have problems running Espresso tests on CI with Thread.sleep(). Idling resources are the solution as mentioned in a comment. I've used on many commercial projects and it's very reliable and doesn't waste unnecessary time sleeping. Imagine if you have 100 tests or 1000 tests full of sleeps, you'll be waiting all day for CI. Not an efficient way to work.

On Wed, 8 Jan 2020 at 12:48, Stephan Kulow notifications@github.com wrote:

@coolo commented on this pull request.

In app/src/main/java/org/piwigo/ui/login/LoginActivity.java https://github.com/Piwigo/Piwigo-Android/pull/224#discussion_r364083770:

     super.finish();

}

 private void loginSuccess(LoginResponse response) {
     fabProgressCircle.hide();
     if (viewModel.isEditExisting()) {
  • startActivity(new Intent(getApplicationContext(), MainActivity.class));

the main view 'eats' all back presses - there is no stack processing

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Piwigo/Piwigo-Android/pull/224?email_source=notifications&email_token=AAB2CGRMKHGXWIJ6X5NKPOTQ4VZKNA5CNFSM4KDGWL72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQ7PTJY#discussion_r364083770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB2CGVBRW5A4ECGXTZTKL3Q4VZKNANCNFSM4KDGWL7Q .

Philio commented 4 years ago

Also, for most use cases, there are already existing solutions: https://developer.android.com/training/testing/espresso/idling-resource#example-implementations

On Wed, 8 Jan 2020 at 13:29, Phil Bayfield mail@phil.io wrote:

You will have problems running Espresso tests on CI with Thread.sleep(). Idling resources are the solution as mentioned in a comment. I've used on many commercial projects and it's very reliable and doesn't waste unnecessary time sleeping. Imagine if you have 100 tests or 1000 tests full of sleeps, you'll be waiting all day for CI. Not an efficient way to work.

On Wed, 8 Jan 2020 at 12:48, Stephan Kulow notifications@github.com wrote:

@coolo commented on this pull request.

In app/src/main/java/org/piwigo/ui/login/LoginActivity.java https://github.com/Piwigo/Piwigo-Android/pull/224#discussion_r364083770 :

     super.finish();

}

 private void loginSuccess(LoginResponse response) {
     fabProgressCircle.hide();
     if (viewModel.isEditExisting()) {
  • startActivity(new Intent(getApplicationContext(), MainActivity.class));

the main view 'eats' all back presses - there is no stack processing

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Piwigo/Piwigo-Android/pull/224?email_source=notifications&email_token=AAB2CGRMKHGXWIJ6X5NKPOTQ4VZKNA5CNFSM4KDGWL72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQ7PTJY#discussion_r364083770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB2CGVBRW5A4ECGXTZTKL3Q4VZKNANCNFSM4KDGWL7Q .

coolo commented 4 years ago

I agree, but at the moment we have 1 test and a buggy app - so sorting this out has priority.

coolo commented 4 years ago

I took out the activity changes - which means the test will fail, but as it doesn't run as part of the CI I would appreciate if you could merge it still and leave the fix of it for a later PR. Because to run it in CI, I need to get the session handling sorted out.

And to test the back behavior, I need to actually fix that too - so I created 2 more PRs. And I would prefer if this didn't run out of scale.