Marcell-Juhasz / secret-diary

0 stars 1 forks source link

maybe extra stages #14

Closed RedJocker closed 2 years ago

RedJocker commented 2 years ago

I see an opportunity in this project for two more stages, one about persistence, so that the content is saved. This could be a stage4. I see stage3 solution already has something going that way so maybe you are struggling with roboletric, and believe me I know what that is like I also struggle myself to learn how to do something I don't already know since there are no sources teaching this stuff. If that is the case let me know I will take a closer look and try to find a solution.

Another possibility is to have a login screen with password so that the content is "secret". This could be a stage5.

But this both are recommendations, this is your call, at least on my part I don't know if people above will complain if there is only 3 stages.

Marcell-Juhasz commented 2 years ago

The two stages you have described already exist with descriptions. The reason why I have written the tests only for the first 3 stages so far is that - as you said - I am not 100% sure if I am writing the tests correctly. By correcting the existing code with your help (that is what we are doing now) I will be able to write the tests for the last two stages without making the same mistakes I did in the first 3. So yes, this is the first time I write tests and I am trying to "learn by doing" through this project. Thank you for your suggestions.

RedJocker commented 2 years ago

oh ok, that is good then. I think currently it is more or less ok, but I haven't spent much time looking for bugs or logic problems, I have only noticed what was more obvious things for me that could improve and reported them here. This project started with the old template, I think the new template can help authors because I prepared some util functions and documented those. Also it helps separating what is not @Tests away from the file that has @Test.

you can see the new template here https://github.com/hyperskill/android-projects-template

and I'm am currently working on another project which I'm actually the one writing the tests although I was not originally the author for the project. You can see me using the new template there. Just ignore the androidTest folders, those were written by the author, but we are not currently running tests with AndroidJUnitRunner because of compatibility issues with EduTools https://github.com/RedJocker/MusicPlayerFix2/tree/addTestStage3/Music%20Player/Stage%203/src/test/java/org/hyperskill/musicplayer

Roboletric is not easy, things are not what they seem to be, with roboletric most of the code that is running are shadows, which are replacements of the actual code. If you don't have that in mind you can spend hours trying to do things that roboletric don't actually do. The thing of roboletric is that it is not running on the device, it is running on the jvm, so it can not actually do "native calls", which you can think of as things done by the device, and that is the importance of the shadow system they use.

Usually there are ways of doing those things, but their documentation is not very informative, so yeah a lot of guessing involved in the process of figuring things out. Some places to start looking are: are there any shadows for the class that is involved in what are you trying to do? Is it something that shadowActivity or activityController can help? Also good to look on the class Roboletric, sometimes there are some helpers there that will solve your problems. Besides that sometimes making searches with pieces of code that you are trying to figure out how to use can help, usually tabnine will have some examples taken from actual existing code, that can hint you on the way you are supposed to go.

If you hit a wall on your tests I recommend opening an issue here and noticing me, I can help figuring out how to do things

Marcell-Juhasz commented 2 years ago

Should I clone the new template to create a new project and follow the instructions below the "Getting Started" section, then copy-paste the tests I have written? Or is it possible to migrate to the new template using the existing project?

RedJocker commented 2 years ago

I did some testing on how to test navigation, you can see it here

https://github.com/RedJocker/hyperskillRoboletricExample

The only thing is that it is leveraging over the AbstractUnitTest that you don't have in your

RedJocker commented 2 years ago

your version is pretty close already, it would be just a matter of including the AbstractUnitTest, but let me update the template, because I used a slightly different version to make easier testing navigation

RedJocker commented 2 years ago

updated the template

Marcell-Juhasz commented 2 years ago

I am using the new template (not pushed to github yet, because it is not working for me).

Finding views don't work with the findViewByString() method declared in the AbstractUnitTest class.

Code in which I am using findViewByString() (instead of the previously used find() method):

private val etNewWriting by lazy { activity.findViewByString("etNewWriting") }

ERROR MESSAGE: View with id "etNewWriting" was not found Same error for all of the views

RedJocker commented 2 years ago

make a new branch with this code, and push this branch here, I will take a look, possibly fix it and make a pull with the correction. then I think it will be cleaner to fix manually the main branch and delete the new branch

RedJocker commented 2 years ago

ooh...are you using testActivity { }, the way it is designed the setup of the activity will be triggered by the testActivity method. I did this method initially to improve error messages when tests are run using the check button with the project already in production. The idea is to make the assertions inside this method

Marcell-Juhasz commented 2 years ago

I don't use that I guess. I have created a new branch so you could see what I did incorrectly.

RedJocker commented 2 years ago

sent a pull request #15, you don't really need to accept since the idea is to delete this branch. Did only stage3 as example, just try following the same pattern for all stages on main branch.

Marcell-Juhasz commented 2 years ago

Done

RedJocker commented 2 years ago

ok, let's try to add the persistence on a stage4 then...if you hit some wall open an issue and I will try to help.

RedJocker commented 2 years ago

added an example for testing persistence with SharedPreferences here

https://github.com/RedJocker/hyperskillRoboletricExample/tree/main/Project%20Name/SharedPreferencesPersistenceExample

If this is not enough for your use case let me know

Marcell-Juhasz commented 2 years ago

Thanks for the example. As I see this test reads the value of the SharedPreferences. But we can not be sure if the user wants to store the values in SharedPreferences unless we explicitly ask them to do that. They may want to use a local or online database.

So I wondered if we should test somehow like this:

  1. Leave some text in the tvDiary that should be stored (do this by performing some Save and Undo actions)
  2. Exit the application, the OnDestroy() function should be called.
  3. Start the application
  4. Check the string in the tvDiary This way, we check if the data is loaded correctly after the application is restarted, not how the user has solved the problem.

What do you think about that idea? And what do you think, could it be implemented in tests? I mean close the application and then restart it.

RedJocker commented 2 years ago

Well I think there are 3 observations I can make regarding this:

1 - I do like this thinking because it is taking into account the concept of black box testing and trying to leave the implementation as open as possible

2 - this project will probably be either easy or medium and that implies that probably users won't know how to persist data and they will expect to have some guidance on that matter on requirements/description. Also topics added are usually based on requirements/description/expectedSolution, including completely new topics.

3 - I don't think we will be able to make that kind of test with roboletric because it is not running on a device so there is no actual persistence being executed.

Having said that I would like to see that being done, so if you happen to find a way that would be interesting, maybe not used for an easy/medium project but used for a challenging one in which users might be close to completing all topics of his track or done that already.

Marcell-Juhasz commented 2 years ago

You're right, the default way to solve this stage is by using SharedPreferences, so that topic will be in the requirements. No other database-related topic will be included in the requirements. So that is why I considered this project a medium-level one. But if the user has some knowledge about databases - maybe they have already learned that on hyperskill to complete another project - they might want to practice that instead of the SharedPreferences that is in this project's requirements. It's up to them.

Referring to your 3rd comment, we will have to navigate between Activities, because in a further stage as you said we will have a LogIn screen. So if we are on the MainActivity and navigating back to LogIn Activity, the OnDestroy() of AvtivityMain will be called. Then we start again the MainActivity from LogIn Activity. So we won't exit the app, but destroy the MainActivity, so the tvDiary is being cleared. And reloaded from persistent storage after relaunching it. But I will take a look if there is another solution for that.

Marcell-Juhasz commented 2 years ago

I have been searching for a solution, how can we simulate closing and relaunching the application.

The first thing came into my mind is do something with the ActivityController, which has start(), stop(), restart(), recreate() etc. functions. Here is the documentation of the class: http://robolectric.org/javadoc/4.3/org/robolectric/android/controller/ActivityController.html

According to documentation, this class provides low-level APIs, therefore it's not recommended to use the functions manually. "Consider using androidx.test.core.app.ActivityScenario instead, which provides higher-level, streamlined APIs to control the lifecycle and it works with instrumentation tests too."

The documentation of the recommended class: https://developer.android.com/reference/androidx/test/core/app/ActivityScenario

What do you think? Should we use this to simulate destroying and recreating the activity for checking whether the persistent data storage works?

RedJocker commented 2 years ago

Read this

https://robolectric.org/extending/

think were would the data be persisted and if it will be

Marcell-Juhasz commented 2 years ago

I have read it and wondering how it can be helpful, but I can not figure it out. What exactly do you mean?

RedJocker commented 2 years ago

When an android app want to persist something it has to use some android code that was made for that purpose, and that android code was made for persisting on mobile devices. RoboletricRunner does not run on a mobile device, it runs on the jvm, those codes that are talking directly to devices either don't run or are intercepted by some other code to do something useful for the test.

If doing a persistence agnostic test is somehow possible it will need a very big hack to make it so. This means you would have to spend many many hours on something that probably won't work in the end.

If you really do want to pursuit such a feature I recommend that you first deliver the tests without this feature, assuming shared preferences is used.

My concern here is that you might be sidetracking into a dead-end that will lead to a burnout of your energy and a big delay in the project.

RedJocker commented 2 years ago

by the way, androidx/test is the espresso library for testing, it is ussually used with AndroidJUnitRunner, that actually runs on a device. We are currently not using AndroidJUnitRunner because of compability issues with EduTools plugin. It is possible to use espresso library with RoboletricRunner, we haven't done that yet, but it might be interesting to see what it can do. But you would still have the same fundamental issue that RoboletricRunner runs on the jvm and not in a mobile device even using espresso library

RedJocker commented 2 years ago

for the lifecycle manipulation, if you really need that, try referencing to this. Look particularly around the section that deals with rotation

https://guides.codepath.com/android/Unit-Testing-with-Robolectric#testing-the-activity-lifecycle

Marcell-Juhasz commented 2 years ago

for the lifecycle manipulation, if you really need that, try referencing to this. Look particularly around the section that deals with rotation

https://guides.codepath.com/android/Unit-Testing-with-Robolectric#testing-the-activity-lifecycle

I have tried this way, but it performs a reassignment for val activityController (declared in the AbstractUnitTest.kt), and val can not be reassigned. If I change it to var, the delegation by 'by' is not working in the declaration. But if you say it won't work because it does not emulate the working of a real Android device, we are there that we should check the content of SharedPreferences and explicitly ask the user to use it.

I also read the AndroidX Test where the scenarios would work more realistically, but I am not enough in testing (this is the first time I write tests) that I could write a class for this that would work properly.

Marcell-Juhasz commented 2 years ago

Another thing came to my mind. If we only check the content of the SharedPreferences, it does not mean that the learner managed to implement the recovery of the stored data successfully. So we must check the diary instead of the value of SharedPreferences.

RedJocker commented 2 years ago

If we only check the content of the SharedPreferences, it does not mean that the learner managed to implement the recovery of the stored data successfully. So we must check the diary instead of the value of SharedPreferences.

yeah, actually there are two different tests here, one to see if the user is changing the state of the sharedPreferences when requiring to save, i think this can be done by looking at the content of the sharedPreferences. A different test is to see if the content of the sharedPrefefrences is being loaded properly when it has to be loaded, here we set the content before setting up the activity and check if the state of the view components is matching this state of the sharedPreferences.

RedJocker commented 2 years ago

I have tried this way, but it performs a reassignment for val activityController (declared in the AbstractUnitTest.kt), and val can not be reassigned. If I change it to var, the delegation by 'by' is not working in the declaration.

can you show me what were you trying? put some extra comments to say what you intended and is not going as expected. To be honest we didn't do much testing involving lifecycle yet. I'm speculating here that something similar to the navigation might work, but I have to see a more concrete case to see what can be done

Marcell-Juhasz commented 2 years ago

I have created a new branch with the non-working code to show you what I am trying to do. To perform the reassignment of the activityController (see: Stage4UnitTest.kt line 310-315) like in the "Simulating Device Rotation" topic, the activityController variable in the AbstractUnitTest.kt (stage4) at line 21 had to be modified from 'val' to 'var'. But this is an error for which the AndroidStudio gives the message: Type 'Lazy<TypeVariable(T)>' has no method 'setValue(AbstractUnitTest<T>, KProperty<*>, ActivityController<T>)' and thus it cannot serve as a delegate for var (read-write property) If it could work somehow, I guess this would be good news because the learner would load the content of the diary either in the onCreate(), onStart(), or onResume() functions I guess. So if we clear the diary as I do in line 312, then recreate the Activity by calling the proper functions, the user's diary-loading function would be called

RedJocker commented 2 years ago

hey how is going? Do you think you have finished writing rests for 4th stage? Have you started 5th stage? Are you having any problems that need help?

The last commit that I can see here was 14 days ago already. If there is something blocking you moving forward please let me know, otherwise just keep on going, I shouldn't be micro-managing you for every step.

Marcell-Juhasz commented 2 years ago

I've completed the code in the last commit as you recommended, so if you agree, I will move to the final stage.

In the 5th stage, the user creates a login Activity for the application. First, we need to check if the login screen works well, then a button navigates to the already implemented diary Activity. Do you have any suggestions or code samples to test this Activity change?

RedJocker commented 2 years ago

Yes you can move to the last stage unless you don't feel that stage4 is ready. Start building the solution as you intend it to work, I can take a look after that. I did one example on navigation a while back that is on the examples repository, but I think it might be simpler than that.

There is one issue that we probably won't be able to solve. Ideally test should run without build failures even if the user has not implemented the solution yet, but we will need to make reference to the class of this different activity on tests, which means build will fail until the user creates the class. If we don't find a work around for this issue we will need to make a warning about that on description.

RedJocker commented 2 years ago

Hey I didn't give you the link to the example that exist on navigation. It is this one: https://github.com/RedJocker/hyperskillRoboletricExample/blob/main/Project%20Name/NavigationExample/src/test/java/org/hyperskill/projectname/Stage1UnitTest.kt.

I think it can be a little simpler than that, probably we will just need to check if the intent is correct and not worry about doing the actual navigation. But for now worry just about the solution, I can take a look myself on that once you have a written solution.

Marcell-Juhasz commented 2 years ago

I've committed my solution for testing the 5th stage. I started from the example you sent and also wrote tests for the cases when the PIN is incorrect

RedJocker commented 2 years ago

Oh, i was expecting only the solution, but if the example worked and you already wrote last stage test that is even better. I think we are close to delivering the project. I will take a closer look now

RedJocker commented 2 years ago

Ok, I sent a pull request with some small changes. I think this project is ok to move forward.

There might be some extra reviewing of descriptions and also we will need to prepare for publishing.

To prepare for publishing you should create a solution branch to hold the code with both tests and solution. Then on the main branch remove the solution from all stages. Stage1 should still have main folder, but it should only have an initial template. All other stages should have only test folder, delete the main folder from them.

Marcell-Juhasz commented 2 years ago

I accepted your pull request and made a new "solution" branch. Then deleted the main folders, except for the first stage I just cleared the MainActivity to a template, and the corresponding layout file also

Marcell-Juhasz commented 2 years ago

No. I'm lost in using git. I try to understand what happened

RedJocker commented 2 years ago

try

git push origin yourSolutionBranchName
Marcell-Juhasz commented 2 years ago

I think now it's working

RedJocker commented 2 years ago

Sent one more pull request to fix the initial template. With this the writing tests phase is probably done.

The project will move to proofreading, someone is going to take a better look on descriptions and prerequisites, if it is all ok it will move to testing.

I will close this issue as soon as you approve the last pull request.

If any problem happens you may open the issue again and contact me.

Marcell-Juhasz commented 2 years ago

Great, I merged your commit to the main branch