Bitstamp/Kraken: Enhance test coverage #1423

Beadko · 2023-12-15T09:20:38Z

PR Description

Fix Bitstamp tests that are failing, improve the test coverage

Fixes # (issue)

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and
also consider improving test coverage whilst working on a certain feature or package.

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

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 Github Actions/AppVeyor with my changes
Any dependent changes have been merged and published in downstream modules

shazbert

Thanks for the update @Beadko 🚀! A linter issue is present as well.

exchanges/kraken/kraken_test.go

exchanges/bitstamp/bitstamp_wrapper.go

exchanges/bitstamp/bitstamp_test.go

shazbert

Thanks for the changes; One pending item left. Looks good!

Also; you do have the option to merge master and not go through the pain of re-applying commits on top of it in a rebase. It's also easier for me to review the steps that occur. But if this is what you like do and you are used to doing it this way, then keep doing it.

exchanges/bitstamp/bitstamp_test.go

shazbert

changes ACK.

shazbert · 2024-01-02T22:23:54Z

@Beadko Merge required.

gloriousCode

Once you merge/rebase master branch, should be good! Thanks!

exchanges/bitstamp/bitstamp_test.go

codecov · 2024-01-16T07:18:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e16ee53) 43.83% compared to head (6d65200) 43.91%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1423      +/-   ##
==========================================
+ Coverage   43.83%   43.91%   +0.08%     
==========================================
  Files         363      363              
  Lines      143963   143963              
==========================================
+ Hits        63107    63228     +121     
+ Misses      73270    73131     -139     
- Partials     7586     7604      +18

see 12 files with indirect coverage changes

thrasher-

Thank you @Beadko for enhancing the test coverage! Some nitterinos found

exchanges/bitstamp/bitstamp_test.go

thrasher-

Thank you for making those changes!

gloriousCode

Hello, I received a panic testing this. After addressing it locally, there still appears to be test issues running the entire package's tests (ie from ./exchanges/bitstamp running go test -race

exchanges/bitstamp/bitstamp_test.go

gloriousCode

tACK! Thanks Beadko for making those changes!

thrasher-

Should be the final batch of nits and then this looks good to merge! Nice stuff

exchanges/bitstamp/bitstamp_test.go

thrasher- · 2024-01-24T06:10:56Z

exchanges/bitstamp/bitstamp_test.go

+		assert.Positive(t, req.High, "High should be positive")
+		assert.Positive(t, req.Low, "Low should be positive")
+		assert.Positive(t, req.Close, "Close should be positive")
+		assert.Positive(t, req.Open, "Low should be positive")


Looks like this one still remains

thrasher- · 2024-01-24T06:15:40Z

exchanges/bitstamp/bitstamp_test.go

 	}
 }

 func TestBitstamp_GetHistoricCandlesExtended(t *testing.T) {
 	pair, err := currency.NewPairFromString("BTCUSD")
 	if err != nil {
-		t.Fatal(err)
+		t.Error("Invalid pair")


Let me know if this is something you were interested to do in this PR or not

….Date

…mp_GetHistoricCandles, TestBitstamp_GetHistoricCandlesExtended

thrasher-

Thanks @Beadko , looking good!

gloriousCode

reTACK

exchanges/kraken/kraken_test.go

shazbert

reTACK

shazbert requested changes Dec 18, 2023

View reviewed changes

shazbert assigned Beadko Dec 18, 2023

shazbert added review me This pull request is ready for review reconstructing Based on PR feedback, this is currently being reworked and is not to be merged labels Dec 18, 2023

Beadko force-pushed the bugfix/bitstamp_fix_failing_tests branch from ae7f069 to 0c40ad7 Compare December 19, 2023 09:04

Beadko requested a review from shazbert December 19, 2023 09:11

shazbert removed the reconstructing Based on PR feedback, this is currently being reworked and is not to be merged label Dec 21, 2023

shazbert requested changes Dec 21, 2023

View reviewed changes

exchanges/bitstamp/bitstamp_test.go Outdated Show resolved Hide resolved

Beadko requested a review from shazbert December 26, 2023 05:49

shazbert added the test(s) fix This is to denote the PR is fixing a build test issue label Dec 27, 2023

shazbert approved these changes Dec 27, 2023

View reviewed changes

shazbert added the szrc shazbert review complete label Dec 27, 2023

thrasher- changed the title ~~Bugfix/bitstamp fix failing tests~~ Bitstamp: Enhance test coverage Dec 29, 2023

shazbert added the rebase/merge of master required label Jan 2, 2024

shazbert removed the szrc shazbert review complete label Jan 2, 2024

gloriousCode requested changes Jan 15, 2024

View reviewed changes

exchanges/bitstamp/bitstamp_test.go Show resolved Hide resolved

Beadko force-pushed the bugfix/bitstamp_fix_failing_tests branch from 67dab95 to 2c36d82 Compare January 16, 2024 07:13

Beadko requested a review from gloriousCode January 16, 2024 07:23

thrasher- requested changes Jan 17, 2024

View reviewed changes

thrasher- removed the rebase/merge of master required label Jan 17, 2024

Beadko force-pushed the bugfix/bitstamp_fix_failing_tests branch from f6a9733 to eb3ba2d Compare January 17, 2024 14:10

Beadko requested a review from thrasher- January 17, 2024 14:11

thrasher- approved these changes Jan 21, 2024

View reviewed changes

gloriousCode requested changes Jan 22, 2024

View reviewed changes

exchanges/bitstamp/bitstamp_test.go Outdated Show resolved Hide resolved

Beadko requested a review from gloriousCode January 22, 2024 13:28

gloriousCode approved these changes Jan 23, 2024

View reviewed changes

thrasher- requested changes Jan 24, 2024

View reviewed changes

Beadko requested a review from thrasher- January 24, 2024 08:29

Beadko added 21 commits February 2, 2024 13:53

Bitstamp: Fix TestWithdrawFiat

ae6ab9e

Bitstamp: Fix TestWithdrawInternationalBank

224bfb6

Bitstamp: Simplify conditions in TestGetDepositAddress

75123bb

Bitstamp: Switch Ws functions Errors to assert.Error

e2ea8c8

Bitstamp: Fix TestSubmitOrder, update other errors

213c50c

Bitstamp: Fix TestGetOrderStatus, amend error comments

f06e4da

Bitstamp: Fix TestGetOrderInfo

8b95b2b

Bitstamp: Fix TestTransferAccountBalance

c9afbf2

Bitstamp: Update TestGetWithdrawalsHistory, fix linter errors

0333e47

Bitstamp: Update TestGetRecentTrades, change type Withdrawal.Requests…

b342bc6

….Date

Bitstamp: Update TestGetDepositAddress, TestBitstamp_OHLC, TestBitsta…

4d94504

…mp_GetHistoricCandles, TestBitstamp_GetHistoricCandlesExtended

Bitstamp: Fix tests

25fe171

Bitstamp: Fix the typos and linter, add a side test to TestGetTicker

da7749b

Bitstamp: Update WithdrawFiat and WithdrawFiatFundsToInternationalBank

1bc8c96

Bitstamp: Fix the error description

057e4ac

Bitstamp: Fixup SkipTestIfCredentialsUnset

bc91cb7

Bitstamp: Fixup tests

322f9e8

Bitstamp: Fix panic, skip live test TestWithdraw, amend mocktest

ae722d4

Bitstamp: Fixup

43b0285

Bitstamp: Fixup

20eec2a

Bitstamp: Fixup

6d65200

Beadko force-pushed the bugfix/bitstamp_fix_failing_tests branch from 5462aa6 to 6d65200 Compare February 2, 2024 06:56

Beadko requested a review from thrasher- February 3, 2024 06:28

thrasher- approved these changes Feb 4, 2024

View reviewed changes

gloriousCode self-requested a review February 4, 2024 22:48

gloriousCode approved these changes Feb 4, 2024

View reviewed changes

thrasher- changed the title ~~Bitstamp: Enhance test coverage~~ Bitstamp/Kraken: Enhance test coverage Feb 4, 2024

thrasher- reviewed Feb 4, 2024

View reviewed changes

exchanges/kraken/kraken_test.go Show resolved Hide resolved

shazbert approved these changes Feb 4, 2024

View reviewed changes

thrasher- merged commit e0c6e11 into thrasher-corp:master Feb 4, 2024
9 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bitstamp/Kraken: Enhance test coverage #1423

Bitstamp/Kraken: Enhance test coverage #1423

Beadko commented Dec 15, 2023

shazbert left a comment

shazbert left a comment

shazbert left a comment

shazbert commented Jan 2, 2024

gloriousCode left a comment

codecov bot commented Jan 16, 2024 •

edited

Loading

thrasher- left a comment

thrasher- left a comment

gloriousCode left a comment

gloriousCode left a comment

thrasher- left a comment

thrasher- Jan 24, 2024

thrasher- Jan 24, 2024

thrasher- left a comment

gloriousCode left a comment

shazbert left a comment

Bitstamp/Kraken: Enhance test coverage #1423

Bitstamp/Kraken: Enhance test coverage #1423

Conversation

Beadko commented Dec 15, 2023

PR Description

Type of change

How has this been tested

Checklist

shazbert left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

shazbert commented Jan 2, 2024

gloriousCode left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 16, 2024 • edited Loading

Codecov Report

thrasher- left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

thrasher- Jan 24, 2024

Choose a reason for hiding this comment

thrasher- Jan 24, 2024

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 16, 2024 •

edited

Loading