offline-agency / laravel-fatture-in-cloud

A wrapper plugin with Fatture in Cloud Api written in Laravel PHP
MIT License
10 stars 8 forks source link

Initial unit test #28

Closed Djuki closed 3 years ago

Djuki commented 4 years ago

@Giacomo92 Please review this initial test.

What I tested is is the auth called with the right API route and post data. I think there is no point in testing the API responses because we actually mock the auth class and API responses.

I am also confused with this line,

Request::info($data);

Looks like this line do not change the request. Can you tell me what is the goal of this line in the Account getInfo method, please?

This is just initial PR, to talk is this correct way to test API routes.

Giacomo92 commented 4 years ago

@Djuki thank again for your work.

I think there is no point in testing the API responses because we actually mock the auth class and API responses. I am agree with you. I think you have to create a stub version of the API response in order to test all Entities.

Request::info($data);

This line of code is used in order to validate data. Every entities have different rules. I have created another issue #24 to improve the data validation.

Can you tell me what is the goal of this line in the Account getInfo method, please? As you can see here the account endpoint provides info about account data and static data used in the FC. You can create a new method for every list returned by the API, so it can be more clear. For example:

     /**
     *
     * @return mixed|string
     */
    public function getBusinessName()
    {
        return $this->auth->post('info/account', $data);
    }
     /**
     *
     * @return mixed|string
     */
    public function getLicenseDuration()
    {
        $data = ['durata_licenza'];
        return $this->auth->post('info/account', $data);
    }
Djuki commented 4 years ago

@Giacomo92 Ok, if I understand correctly:

Do you think that this test can be improved? Is there something else you think we can cover in this test?

I will update the PR with your code styles. And also I will update this PR with more tests when we make this test perfect for you.

Giacomo92 commented 4 years ago

@Djuki

request class shouldn't change the input data

Yes

request should throw an exception if data is not correct

Yes

I think you should mock all the response in order to cover all Entities. Check also the CI log as you can see down here some checks were not successful