From 1edfb8942385f5afc701829165ac237dbe49fb14 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 16 Jul 2018 12:30:54 -0700 Subject: [PATCH 1/4] contribution guide: Add guidelines for testing Indicates to use require's and asserts, and to use table driven tests, with error messages as described in #1664. Closes #1664 --- CONTRIBUTING.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0fcf36def..c04382391 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,6 +67,29 @@ tested by circle using `go test -v -race ./...`. If not, they will need a `circle.yml`. Ideally, every repo has a `Makefile` that defines `make test` and includes its continuous integration status using a badge in the `README.md`. +We expect tests to use `require` or `assert` rather than `t.Skip` or `t.Fail`, +unless there is a reason to do otherwise. +We prefer to use [table driven tests](https://github.com/golang/go/wiki/TableDrivenTests) +where applicable. +Error messages should follow the following format +`, tc #, i #`. +`` is an optional short description of whats failing, `tc` is the +index within the table of the testcase that is failing, and `i` is when there +is a loop, exactly which iteration of the loop failed. +The idea is you should be able to see the +error message and figure out exactly what failed. +Here is an example check: + +``` + +for tcIndex, tc := range cases { + + for i := 0; i < tc.numTxsToTest; i++ { + + require.Equal(t, expectedTx[:32], calculatedTx[:32], + "First 32 bytes of the txs differed. tc #%d, i #%d", tcIndex, i) + ``` + ## Branching Model and Release User-facing repos should adhere to the branching model: http://nvie.com/posts/a-successful-git-branching-model/. From 2b72e3377a4177ed3d1153c0a4ff4120f2ac12c0 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 16 Jul 2018 17:24:07 -0700 Subject: [PATCH 2/4] Clarify when to use table driven tests --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c04382391..b18ef2ea9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,9 +69,9 @@ includes its continuous integration status using a badge in the `README.md`. We expect tests to use `require` or `assert` rather than `t.Skip` or `t.Fail`, unless there is a reason to do otherwise. -We prefer to use [table driven tests](https://github.com/golang/go/wiki/TableDrivenTests) -where applicable. -Error messages should follow the following format +When testing a function under a variety of different inputs, we prefer to use +[table driven tests](https://github.com/golang/go/wiki/TableDrivenTests). +Table driven test error messages should follow the following format `, tc #, i #`. `` is an optional short description of whats failing, `tc` is the index within the table of the testcase that is failing, and `i` is when there From d6969c1d221387a7640b741873bfe9161299ee9a Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 17 Jul 2018 13:59:06 -0700 Subject: [PATCH 3/4] Merge PR #1697: Proposal Query filter by status --- CHANGELOG.md | 11 +++++++++++ Gopkg.lock | 4 +--- client/lcd/lcd_test.go | 22 +++++++++++++++++++++- x/gov/client/rest/rest.go | 28 ++++++++++++++++++++++++---- x/slashing/params.go | 2 +- x/slashing/test_common.go | 2 +- x/stake/types/msg.go | 2 +- 7 files changed, 60 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29d753a6d..81b65ed37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## TBD + +BREAKING CHANGES + +FEATURES +* [lcd] Can now query governance proposals by ProposalStatus + +IMPROVEMENTS + +BUG FIXES + ## 0.22.0 *July 16th, 2018* diff --git a/Gopkg.lock b/Gopkg.lock index 63f53a96f..66f6c0492 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2,7 +2,6 @@ [[projects]] - branch = "master" name = "github.com/bartekn/go-bip39" packages = ["."] revision = "a05967ea095d81c8fe4833776774cfaff8e5036c" @@ -135,7 +134,6 @@ ".", "hcl/ast", "hcl/parser", - "hcl/printer", "hcl/scanner", "hcl/strconv", "hcl/token", @@ -503,6 +501,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "94abff3ff321fd150a6e4b95d109297296cdc00693c648c9b2a48171b90e36b0" + inputs-digest = "71e86b1f1e9ec71901c20d8532dc8477df66eff37a407322379f6a8b03e5d91b" solver-name = "gps-cdcl" solver-version = 1 diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 9180af87a..f5b2d263e 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -569,6 +569,16 @@ func TestProposalsQuery(t *testing.T) { resultTx = doDeposit(t, port, seed2, name2, password2, addr2, proposalID3) tests.WaitForHeight(resultTx.Height+1, port) + // Only proposals #1 should be in Deposit Period + proposals := getProposalsFilterStatus(t, port, gov.StatusDepositPeriod) + require.Len(t, proposals, 1) + require.Equal(t, proposalID1, proposals[0].GetProposalID()) + // Only proposals #2 and #3 should be in Voting Period + proposals = getProposalsFilterStatus(t, port, gov.StatusVotingPeriod) + require.Len(t, proposals, 2) + require.Equal(t, proposalID2, proposals[0].GetProposalID()) + require.Equal(t, proposalID3, proposals[1].GetProposalID()) + // Addr1 votes on proposals #2 & #3 resultTx = doVote(t, port, seed, name, password1, addr, proposalID2) tests.WaitForHeight(resultTx.Height+1, port) @@ -580,7 +590,7 @@ func TestProposalsQuery(t *testing.T) { tests.WaitForHeight(resultTx.Height+1, port) // Test query all proposals - proposals := getProposalsAll(t, port) + proposals = getProposalsAll(t, port) require.Equal(t, proposalID1, (proposals[0]).GetProposalID()) require.Equal(t, proposalID2, (proposals[1]).GetProposalID()) require.Equal(t, proposalID3, (proposals[2]).GetProposalID()) @@ -910,6 +920,16 @@ func getProposalsFilterVoterDepositer(t *testing.T, port string, voterAddr, depo return proposals } +func getProposalsFilterStatus(t *testing.T, port string, status gov.ProposalStatus) []gov.Proposal { + res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals?status=%s", status), nil) + require.Equal(t, http.StatusOK, res.StatusCode, body) + + var proposals []gov.Proposal + err := cdc.UnmarshalJSON([]byte(body), &proposals) + require.Nil(t, err) + return proposals +} + func doSubmitProposal(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress) (resultTx ctypes.ResultBroadcastTxCommit) { // get the account to get the sequence acc := getAccount(t, port, proposerAddr) diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 147304587..ffaf42749 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -16,10 +16,11 @@ import ( // REST Variable names // nolint const ( - RestProposalID = "proposalID" - RestDepositer = "depositer" - RestVoter = "voter" - storeName = "gov" + RestProposalID = "proposalID" + RestDepositer = "depositer" + RestVoter = "voter" + RestProposalStatus = "status" + storeName = "gov" ) // RegisterRoutes - Central function to define routes that get registered by the main application @@ -340,10 +341,12 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { bechVoterAddr := r.URL.Query().Get(RestVoter) bechDepositerAddr := r.URL.Query().Get(RestDepositer) + strProposalStatus := r.URL.Query().Get(RestProposalStatus) var err error var voterAddr sdk.AccAddress var depositerAddr sdk.AccAddress + var proposalStatus gov.ProposalStatus if len(bechVoterAddr) != 0 { voterAddr, err = sdk.AccAddressFromBech32(bechVoterAddr) @@ -365,6 +368,16 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { } } + if len(strProposalStatus) != 0 { + proposalStatus, err = gov.ProposalStatusFromString(strProposalStatus) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + err := errors.Errorf("'%s' is not a valid Proposal Status", strProposalStatus) + w.Write([]byte(err.Error())) + return + } + } + ctx := context.NewCoreContextFromViper() res, err := ctx.QueryStore(gov.KeyNextProposalID, storeName) @@ -397,9 +410,16 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { if err != nil || len(res) == 0 { continue } + var proposal gov.Proposal cdc.MustUnmarshalBinary(res, &proposal) + if len(strProposalStatus) != 0 { + if proposal.GetStatus() != proposalStatus { + continue + } + } + matchingProposals = append(matchingProposals, proposal) } diff --git a/x/slashing/params.go b/x/slashing/params.go index b0c85698c..45d2833ce 100644 --- a/x/slashing/params.go +++ b/x/slashing/params.go @@ -70,7 +70,7 @@ var ( // TODO Temporarily set to 10 minutes for testnets defaultDowntimeUnbondDuration int64 = 60 * 10 - defaultMinSignedPerWindow sdk.Rat = sdk.NewRat(1, 2) + defaultMinSignedPerWindow = sdk.NewRat(1, 2) defaultSlashFractionDoubleSign = sdk.NewRat(1).Quo(sdk.NewRat(20)) diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 5cec7ce78..190932995 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -34,7 +34,7 @@ var ( sdk.AccAddress(pks[1].Address()), sdk.AccAddress(pks[2].Address()), } - initCoins sdk.Int = sdk.NewInt(200) + initCoins = sdk.NewInt(200) ) func createTestCodec() *wire.Codec { diff --git a/x/stake/types/msg.go b/x/stake/types/msg.go index 08fd80128..27edad5dd 100644 --- a/x/stake/types/msg.go +++ b/x/stake/types/msg.go @@ -21,7 +21,7 @@ var _, _ sdk.Msg = &MsgBeginUnbonding{}, &MsgCompleteUnbonding{} var _, _ sdk.Msg = &MsgBeginRedelegate{}, &MsgCompleteRedelegate{} // Initialize Int for the denominator -var maximumBondingRationalDenominator sdk.Int = sdk.NewInt(int64(math.Pow10(MaxBondDenominatorPrecision))) +var maximumBondingRationalDenominator = sdk.NewInt(int64(math.Pow10(MaxBondDenominatorPrecision))) //______________________________________________________________________ From f88d64499dba0f2e57cc52d10ab22ce0aef5247e Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Tue, 17 Jul 2018 14:11:34 -0700 Subject: [PATCH 4/4] Merge PR #1705: baseapp: Allow alphanumerics in routes Previously only alphabetic characters were allowed. --- CHANGELOG.md | 3 ++- baseapp/baseapp_test.go | 2 +- baseapp/router.go | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81b65ed37..ef203bbd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## TBD +## PENDING BREAKING CHANGES @@ -8,6 +8,7 @@ FEATURES * [lcd] Can now query governance proposals by ProposalStatus IMPROVEMENTS +* [baseapp] Allow any alphanumeric character in route BUG FIXES diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 00897392e..b2e0760ea 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -290,7 +290,7 @@ func (tx txTest) GetMsgs() []sdk.Msg { return tx.Msgs } const ( typeMsgCounter = "msgCounter" - typeMsgCounter2 = "msgCounterTwo" // NOTE: no numerics (?) + typeMsgCounter2 = "msgCounter2" ) // ValidateBasic() fails on negative counters. diff --git a/baseapp/router.go b/baseapp/router.go index abbbf9e12..4be3aec74 100644 --- a/baseapp/router.go +++ b/baseapp/router.go @@ -31,12 +31,12 @@ func NewRouter() *router { } } -var isAlpha = regexp.MustCompile(`^[a-zA-Z]+$`).MatchString +var isAlphaNumeric = regexp.MustCompile(`^[a-zA-Z0-9]+$`).MatchString // AddRoute - TODO add description func (rtr *router) AddRoute(r string, h sdk.Handler) Router { - if !isAlpha(r) { - panic("route expressions can only contain alphabet characters") + if !isAlphaNumeric(r) { + panic("route expressions can only contain alphanumeric characters") } rtr.routes = append(rtr.routes, route{r, h})