BratinaRok / android-bank-manager

bank manager application
0 stars 1 forks source link

creating tests #1

Open RedJocker opened 2 years ago

RedJocker commented 2 years ago

Hey I will be doing code revision for your project, use this thread to make me questions
I recently started this repository with roboletric examples, if it does not solve your doubts in a particular issue am here for that. if it is an easy to solve issue I can just answer here, if it is a more complicated I can upload a new example.

There are also some tests that come with the template, they should run even though you have changed the application and that is something important that we have to keep, our tests should avoid assuming things from the implementations whenever possible, so don't call methods from implementation code directly on your tests. Think the code you are testing as a black-box.

If you read the test code that was in the template you will notice that there are class fields with activity views using lazy initialization and some assertions about the expected initial state of the view. This is because we should not be assuming tests will run in any specific order, junit won't ensure us that. But junit will reinstatiate fields for each @Test method, so you don't have to worry about cleaning state between @Tests .

on @Tests you will see most of the test is done inside a method called testActivity. This testActivity will be doing the setup of the activity with roboletric, so before it the activity don't exist yet if there is some setup needed before launching the activity it happens before testActivity.

inside testActivity the first thing to do is to initialize the lazy fields you are going to use on your @Test so that they initialize with the initial state of the activity. After that the usual testing pattern is setting up a particular state, making assertions about that state, changing state, making assertions, changing state, making assertions. But you should prefer making more @Test with with fewer rounds of assertions than making a few very big @test with multiple rounds of assertions.

Make some reading of the AbstractUnitTest, it will provide some useful methods for testing and there is also some useful informations on the documentation of the class

One last thing use findViewByString() instead of findViewById, if we reference the id through R.id.someView the test code won't compile if that id was not declared.

BratinaRok commented 2 years ago

Hello, I finished tests for stage 1, please have a look. https://github.com/BratinaRok/android-bank-manager/blob/stage1Tests/Bank%20Manager/stage1/src/test/java/org/hyperskill/bankmanager/Stage1UnitTest.kt

RedJocker commented 2 years ago

can you make some description? it doesn't need to be a production ready description, it is just to help me understand what the stage is about

RedJocker commented 2 years ago

I'm getting build failure, I believe the problem is that the packages in your src/main/ are all package com.example.myapplication which is not right.

edit: just realized that androidStudio dowloaded the main branch

RedJocker commented 2 years ago

ok, I already created a bunch of issues to be fixed, but I didn't really paid much attention to the tests because I don't really know what we are trying to accomplish on this stage. I can see that this solution code is probably copied from a full implementation with some omissions, so it is hard to make inferences on what is specific to this stage and what isn't. I will be able to take a better look on tests after some description is written so I understand what we should be testing

RedJocker commented 2 years ago

Also I should probably say that is normal to have issues, so don't worry. This project seems to be going for either Hard or Challenging and there is use of a lot of content that is not on topics yet, but that is a problem for the content team not yours. My initial impression on the project is actually a good one.

BratinaRok commented 2 years ago

Hello, for description at stage 1, please check readme

RedJocker commented 2 years ago

hi, I've just seen your update, I'll take a look on it soon. One simple thing to making reviewing easier is to put on the commit message the number of the issue you were dealing with on that commit. for example, let's say you added tests cases for stage 2 on your commit you could have a message like "add stage2 test cases #1" and then I would know you were working on issue number 1 and also github would link the commit to the issue. You can also put as many as you would like so something like "add stage2 test cases #1 #3 #11" would reference the commit with issues 1 3 and 11.

If you look now at issue 3 you will see a reference to this comment here because comments on issues about other issues also get referenced the same way

RedJocker commented 2 years ago

Hello, for description at stage 1, please check readme

if you prefered to write on the readme because of markdown syntax, you can rename the task.html to task.md and use the markdown on there. make descriptions for stages on the task file of the stage

BratinaRok commented 2 years ago

Please check task.html, there is whole description for stage 1

BratinaRok commented 2 years ago

Hi, I am working on tests for stage2 and I need some help with a test to check that after successful signup, the app redirects back to the main screen. How can I check that the app shows main screen?

RedJocker commented 2 years ago

we haven't work with navGraph before, but try looking for widgets that are specific from the main screen. If looking for them in the activity does not work try looking for them in the fragment container. My guess is that if you look for a widget that is not in the current fragment the test will fail and if you look for a widget that is on the current fragment you will get the widget back

RedJocker commented 2 years ago

ok, code on stage 2 tests seem interesting, I will take a closer look at that later on, but abstracting common patterns is usually good.

Description look ok too, I will check if it match tests when I take the closer look i just said above.

BratinaRok commented 2 years ago

Hi, sorry for the late response, I am working on fixing the stages

RedJocker commented 2 years ago

No problem, you can take your time. If you have some problem to go forward you can warn me.

BratinaRok commented 2 years ago

If for example wrong ID is added to some button and test fails with error:

View with id "manLogInButton" was not found
java.lang.AssertionError: View with id "manLogInButton" was not found

Can I throw some custom message, so that it will be known what the correct result is? I already tried with this :

fun checkIfExists() {
                val message = "Log in button doesn't exists at main screen"
                Assert.assertNotNull(message ,activity.findViewByString("mainLogInButton"))
            }
RedJocker commented 2 years ago

that assertion is being done on internals/AbstractUnitTest.findViewByString(), but it does not accept a custom message. I can write one that does though, if you need it

RedJocker commented 2 years ago

I have written it, create a branch from main named alternativeSolution I will make a pull request with that code and the code with the solution fixes I mentioned on another comment

BratinaRok commented 2 years ago

Hi, I just created alternativeSolution branch

RedJocker commented 2 years ago

Oh no, I said main but it was supposed to be master, now I think I have lost all the code I had written trying to sync branchs to make the pull request.

please delete this branch and make again from master.

I will write the changes again later

BratinaRok commented 2 years ago

Ok, I made alternativeSolution branch from master. Should I set default as master and delete main so it will be easier later to make changes?

RedJocker commented 2 years ago

I think so, I don't see any need for this main branch if it is not being used

RedJocker commented 2 years ago

I was able to recover changes I had done on the tests, because I had also committed them on master branch, but solution changes are lost for now. I will make the pull request with the test changes only, if I have some time I will make the changes on solution again later.

BratinaRok commented 2 years ago

Great, thanks. I deleted main so now only master exists and branch.

RedJocker commented 2 years ago

There are several open issues for this project and I don't know if you have solved them or not. Keep in mind that I'm paid by the hour and I should not be spending too much money on a project otherwise it is not worth it.

Help me help you and inform what have you done to solve the open issues and if you think they are solved or not.

The conventional way to do that is to reference the issue number in the commit so that I know which commit the issue has been solved. But I think that is beyond the point now, just write me an update on the status of every open issue.

The project won't be considered done while still maintaining many open issues. Ideally there should be none, one or another in exceptional cases