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

Separate unit tests and integration tests #1806

Closed
4 tasks
greg-szabo opened this issue Jul 24, 2018 · 2 comments
Closed
4 tasks

Separate unit tests and integration tests #1806

greg-szabo opened this issue Jul 24, 2018 · 2 comments

Comments

@greg-szabo
Copy link
Member

Summary

Unit tests don't require the built binary, they can be run against the source code using go test.
Integration tests require the built binary and they will fail if a binary is not found. Such tests include TestGaiaCLICreateValidator, TestGaiaCLISubmitProposal, and TestGaiaCLISubmitProposal.

Currently, the only distinction is that some tests that require the binary are manually separated in the Makefile with the PACKAGES_NOCLITEST variable.

Please create a standard set of names for the tests that can be prefixed to any test function (for example: TestSimple, TestIntegration or some other sort, like based on package name: TestGaiad... TestGaiacli...) so the -run option becomes useful in running go test.

Problem Definition

go test will fail with a freshly cloned or pulled repository because the binary was not built. Also, the path of the binary cannot be specified, it is hard-coded in the tests. This makes any kind of automated or custom testing very hard.
Currently, there are 219 different test names in the repository and 22 of the alphabet's letters are used as first characters. This means, that if I want to exclude the CLI-related tests, I have to invoke almost all letters of the alphabet, like this:
go test ./... -run '([ABCDEFHIKLMNOPQRSTUVZ]*|GaiaApp.*|Gas.*|Ge.*)'

What problems may be addressed by introducing this feature?
Stability of testing is improved. More options available for testing.

What benefits does the SDK stand to gain by including this feature?
Testing under different circumstances is essential for a stable code. Partial testing will be easier.

Are there any disadvantages of including this feature? -->
No. Current CircleCI testing already excludes integration testing code. The test_cli target is already separated in the Makefile. Current functionality will remain and new functionality becomes avaiable.

Proposal

Name the test functions alike and use the -run option in go test. For example: go test -run Bonding.

The following name prefixes are only used once. Giving them a prefix that was already used before cuts down on one-off tests:

AccountMapperGetSet
AmountOf
Arithmatic
BeginBlocker
Block
CheckTx
ClearTendermintUpdates
CodeType
CoolKeeper
DeliverTx
DuplicatesMsgCreateValidator
EmbeddedStructSerializationGoWire
EmptyTags
Equalities
ErrFn
Evaluate
FromInt64
FullValidatorSetPowerChange
Genesis
GenTxCmd
InflationWithRandomOperations
Info
InputValidation
Int
Keys
ListMapper
LoadVersion
LogContext
MinusCoin
MountStores
NodeStatus
OutputValidation
P2PQuery
Params
Pool
PossibleOverflow
PrefixEndBytes
ProcessProvisions
Query
QueueMapper
RatsEqual
RevokeValidator
Round
RunInvalidTransaction
SameDenomAsCoin
SendKeeper
SengMsgMultipleInOut
SetValidator
SimulateTx
SingleValidatorIntegrationInvariants
SlashingMsgs
SortCoins
StakeMsgs
StdTx
Store
TestnetFilesCmd
Uint
Version
ZeroSerializationJSON

In comparison, the most used prefixes are:

Msg
New
Gaia
Validator

We hardly use Get or Set as a prefix for tests.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ValarDragon
Copy link
Contributor

As an alternative, we could put the CLI tests behind a build flag, in the same vein as in #1665

@greg-szabo
Copy link
Member Author

Yep, that looks even better.

ValarDragon added a commit that referenced this issue Jul 25, 2018
This is done to make go test ./... work for people using the sdk as a sdk.
Closes #1806
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants