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

fixes #279 Continue improving coverage #320

Merged
merged 89 commits into from
Feb 4, 2020

Conversation

e1Ru1o
Copy link
Contributor

@e1Ru1o e1Ru1o commented Jan 24, 2020

fixes #279

Changes:

  • Add more tests mainly related with files not covered by the actual test suite
  • Move mock types for internal Skycoin types onto src/coin/skycoin/skymocks

Does this change need to mentioned in CHANGELOG.md?
no

Requires testing
no, indeed, new tests added

AntiD2ta and others added 30 commits January 3, 2020 19:13
…stination and a Combo box to select log level
I would like a ComboBox with fixed options, but I couldn't do it right
This allows to changes logger properties and propagate this changes across all modules's logger
Changes:
- Add SetOutput function to set the desired logger's output given a option
- Return error in SetOutputToFile function
Remove SetOutput function and add GetOutputWriter function to return the io.Writer that needs to be given to SetOutputTo function
…ject's logger

This occurs when the app's logger change its output
…age' into stdevAntiD2ta_t298_Log_events_to_file
Bug:
Return Stderr writer instead of Stdout when option is stdout
e1Ru1o and others added 19 commits January 25, 2020 20:37
Changes:
-  `TestTransactionIsFullySigned` changed to the new mentioned test with the purpose of be generic
`TestTransactionVerifyUnsigned` and `TestTransactionVerifySigned`
…oins` to `TestGetCoins`

Make the test more generic. Every object with `GetCoins` method could be tested using it
Remove other `GetId` test, make a new one generic for all object that have a method `GetId`
@@ -0,0 +1,36 @@
// Code generated by mockery v1.0.0. DO NOT EDIT.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal interface . No mocks needed .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage instances should be replaced with concrete type implementing the internal interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage instances should be replaced with concrete type implementing the internal interface.

Forget about this previous comment .

The only thing needed is to move this class somewhere under src/coin/skycoin/ sub package tree (e.g. src/coin/skycoin/skymocks) . Code under src/coin/mocks should correspond to types defined in src/core .

}

func TestEncodeSkycoinTransaction(t *testing.T) {
type SkycoinReadableTxn interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to define this . Use individual types .

@@ -0,0 +1,36 @@
// Code generated by mockery v1.0.0. DO NOT EDIT.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage instances should be replaced with concrete type implementing the internal interface.

Forget about this previous comment .

The only thing needed is to move this class somewhere under src/coin/skycoin/ sub package tree (e.g. src/coin/skycoin/skymocks) . Code under src/coin/mocks should correspond to types defined in src/core .

Copy link
Contributor

@olemis olemis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a rule in Makefile for generating mocks for internal Skycoin interfaces and place them under src/coin/skycoin subfolder tree .

@olemis olemis changed the title Refs #279 Continue improving coverage fixes #279 Continue improving coverage Feb 4, 2020
@olemis olemis merged commit 2d208ba into fibercrypto:develop Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuos Integration stdev wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve coverage in develop branch
4 participants