Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(QOL) Improve test setup for logger, improve test coverage for database #447

Merged
merged 6 commits into from Feb 17, 2020

Conversation

@xtda
Copy link
Member

xtda commented Feb 14, 2020

PR Description

Due to the way code coverage is calculated some tests in the database package were not counting towards it.

This PR creates a seperate testhelpers package (not imported by any non-test packages so not built into binary) that moves reusable functions into that and moves tests for database into seperate packages.

Also cleans up logger package test setup and fixes a test bug

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested

  • go test ./... -race
  • golangci-lint run

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Travis with my changes
  • Any dependent changes have been merged and published in downstream modules
xtda added 3 commits Feb 14, 2020
@xtda xtda self-assigned this Feb 14, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 14, 2020

Codecov Report

Merging #447 into master will decrease coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
- Coverage   41.26%   40.98%   -0.28%     
==========================================
  Files         197      200       +3     
  Lines       43105    43284     +179     
==========================================
- Hits        17788    17741      -47     
- Misses      23912    24074     +162     
- Partials     1405     1469      +64     
Impacted Files Coverage Δ
exchanges/gemini/gemini.go 80.55% <0.00%> (-8.34%) ⬇️
exchanges/lbank/lbank.go 26.57% <0.00%> (-5.15%) ⬇️
exchanges/localbitcoins/localbitcoins.go 48.63% <0.00%> (-4.26%) ⬇️
exchanges/binance/binance.go 76.63% <0.00%> (-3.99%) ⬇️
exchanges/poloniex/poloniex_wrapper.go 46.19% <0.00%> (-3.05%) ⬇️
exchanges/poloniex/poloniex.go 38.51% <0.00%> (-2.77%) ⬇️
exchanges/binance/binance_wrapper.go 53.22% <0.00%> (-2.39%) ⬇️
exchanges/bitstamp/bitstamp.go 80.60% <0.00%> (-2.34%) ⬇️
exchanges/gemini/gemini_wrapper.go 43.02% <0.00%> (-2.33%) ⬇️
exchanges/bitstamp/bitstamp_wrapper.go 46.49% <0.00%> (-1.87%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72658dc...38a17c5. Read the comment docs.

ConnectionDetails: drivers.ConnectionDetails{
Host: "localhost",
Port: 5432,
Username: "andrew",

This comment has been minimized.

Copy link
@thrasher-

thrasher- Feb 15, 2020

Collaborator

Although I'd' vote Andrew for president, how about gctuser

This comment has been minimized.

Copy link
@xtda

xtda Feb 15, 2020

Author Member

will remove these not meant to be there as user should self configure

Copy link
Collaborator

thrasher- left a comment

Apart from that single comment, the rest looks good! Good stuff 👍

Copy link
Collaborator

gloriousCode left a comment

tACK! test helpers will be great moving forward

Copy link
Collaborator

shazbert left a comment

tACK 👍

Copy link
Collaborator

MadCozBadd left a comment

LGTM!

@thrasher- thrasher- merged commit f515a2b into thrasher-corp:master Feb 17, 2020
2 of 5 checks passed
2 of 5 checks passed
CodeLingo timed out
Details
Travis CI - Pull Request Build Failed
Details
codecov/project 40.98% (+-0.28%) compared to 72658dc
Details
codecov/patch 58.82% of diff hit (target 41.26%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@xtda xtda deleted the xtda:misc-fixes branch Feb 19, 2020
Christian-Achilli pushed a commit to Christian-Achilli/gocryptotrader that referenced this pull request Feb 21, 2020
…se (thrasher-corp#447)

* Improved test setup for logger, improved test coverage for database

* removed some new lines

* add new line

* removed database config detailS

* removed lines

* code cleanup
Christian-Achilli pushed a commit to Christian-Achilli/gocryptotrader that referenced this pull request Feb 21, 2020
…se (thrasher-corp#447)

* Improved test setup for logger, improved test coverage for database

* removed some new lines

* add new line

* removed database config detailS

* removed lines

* code cleanup
thrasher- added a commit that referenced this pull request Feb 24, 2020
* fix 440: SubmitOrder and CB-ACCESS-TIMESTAMP value

* removed gctcli executable and added it to gitignore

* removed nonce from conibasepro send payload

* fixed typo in executable name

* Bump github.com/grpc-ecosystem/grpc-gateway from 1.12.2 to 1.13.0 (#442)

Bumps [github.com/grpc-ecosystem/grpc-gateway](https://github.com/grpc-ecosystem/grpc-gateway) from 1.12.2 to 1.13.0.
- [Release notes](https://github.com/grpc-ecosystem/grpc-gateway/releases)
- [Changelog](https://github.com/grpc-ecosystem/grpc-gateway/blob/master/CHANGELOG.md)
- [Commits](grpc-ecosystem/grpc-gateway@v1.12.2...v1.13.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Minor general cleanup and bug fixes (#443)

* Bugfix, remove non-needed code and cleanup some code

* Run go mod tidy

* Remove non-needed test and fix tautological err

* Fix Huobi interim var reference

* (Logger) Rename package to log (#444)

* renamed package to log to stop side import requirement

* reverted comment changes

* reverted comment changes

* one more reverting wording back to logger

* wording changes on comments

* Bump github.com/gorilla/mux from 1.7.3 to 1.7.4 (#445)

Bumps [github.com/gorilla/mux](https://github.com/gorilla/mux) from 1.7.3 to 1.7.4.
- [Release notes](https://github.com/gorilla/mux/releases)
- [Commits](gorilla/mux@v1.7.3...v1.7.4)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* (QOL) Improve test setup for logger, improve test coverage for database (#447)

* Improved test setup for logger, improved test coverage for database

* removed some new lines

* add new line

* removed database config detailS

* removed lines

* code cleanup

* Bump github.com/d5/tengo/v2 from 2.0.2 to 2.0.4 (#449)

Bumps [github.com/d5/tengo/v2](https://github.com/d5/tengo) from 2.0.2 to 2.0.4.
- [Release notes](https://github.com/d5/tengo/releases)
- [Changelog](https://github.com/d5/tengo/blob/master/.goreleaser.yml)
- [Commits](d5/tengo@v2.0.2...v2.0.4)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Bump github.com/d5/tengo/v2 from 2.0.4 to 2.0.5 (#450)

Bumps [github.com/d5/tengo/v2](https://github.com/d5/tengo) from 2.0.4 to 2.0.5.
- [Release notes](https://github.com/d5/tengo/releases)
- [Changelog](https://github.com/d5/tengo/blob/master/.goreleaser.yml)
- [Commits](d5/tengo@v2.0.4...v2.0.5)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Adrian Gallagher <thrasher@addictionsoftware.com>
Co-authored-by: Andrew <andrew@disvelop.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.