Skip to content

Make testable#8

Closed
chrisjoyce911 wants to merge 12 commits intoRocketChat:masterfrom
chrisjoyce911:make_testable
Closed

Make testable#8
chrisjoyce911 wants to merge 12 commits intoRocketChat:masterfrom
chrisjoyce911:make_testable

Conversation

@chrisjoyce911
Copy link

I have done a large refactor to make development and testing easier.

Why have I made changes ?

sub-packages make it hard ! Having models in a sub-package makes it hard under the golang dependancy management to fork and work on changes, moving all in a single package solves this.

make testing easy I have added a http doer to the client , this allows for testing to include the response that you want to test with.

its a work in progress I have done a lot of changes, and some are still a bit messy. More work is needed

started to lint I find lint'ing of code very helpful make lint

test coverage Have also added a compact way to look at test coverage make test

Keen for your feedback,

Chris

Chris Joyce and others added 12 commits August 25, 2018 20:41
Was
 POST https://rocketchat.server/api/v1/login HTTP/1.1 1 1 map[Content-Type:[application/json]] {password=PASSWORD&user=USER}

Now
 POST https://rocketchat.server/api/v1/login HTTP/1.1 1 1 map[Content-Type:[application/json]] {{"username":"USER","password":"PASSWORD"}}
Using multi pagkages makes forked development very hard,
moved things round to have all in single pagkage

* Added Doer - can pass this in
* REST channels has good coverage
* REST information has good coverage
Added missing tests for REST
Included error tests
Added missing tests for REST
Included error tests
Renamed channels tests
Extended the example to include a PostMessage
Most client methods are used by other func during tests
Very simple tests only
Login and Logout
I have left some dead/unsed code that needs cleanup
@geekgonecrazy
Copy link
Contributor

First off thanks for your contribution!

sub-packages make it hard ! Having models in a sub-package makes it hard under the golang dependancy management to fork and work on changes, moving all in a single package solves this.

Strongly disagree with this. By putting in a subpackage you are allowing that one package to be included where you may not want to include the entire sdk here. Its fundamental to go.


Going to take a look at the rest of the code changes.

Thanks again for contributing

@chrisjoyce911
Copy link
Author

I have created another pull request that contains the change to make testing easer. (#13)
Closing this request

@chrisjoyce911 chrisjoyce911 deleted the make_testable branch August 29, 2018 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants