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

Add CLI tests for simd, distribution #6095

Merged
merged 88 commits into from
May 1, 2020

Conversation

sahith-narahari
Copy link
Contributor

Reference - #5951

Description

This PR adds tests for simd cli commands and distribution commands


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

sahith-narahari and others added 30 commits April 15, 2020 03:57
…esh/cli-test-send

# Conflicts:
#	cli_test/tests/staking_test.go
@alessio alessio added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 30, 2020
alessio
alessio previously approved these changes Apr 30, 2020
Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Brilliant, thanks!

@alessio alessio dismissed their stale review April 30, 2020 23:22

Tests are failing

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Tests failed again:

--- FAIL: TestValidateGenesis (0.00s)
    gobash.go:60: Running simd --home=/tmp/sdk_integration_TestValidateGenesis_897338208/.simd unsafe-reset-all
    gobash.go:72: 
        	Error Trace:	gobash.go:72
        	            				executors.go:22
        	            				executors.go:17
        	            				helpers.go:47
        	            				init.go:18
        	            				simd_test.go:118
        	Error:      	Received unexpected error:
        	            	exec: "simd": executable file not found in $PATH
        	Test:       	TestValidateGenesis

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Left few suggestions. I'm also attaching a patch to fix the issue - I hope this helps:

commit a64ecea3c67c4fde234b092a29627279e2a4af41
Author: Alessio Treglia <alessio@tendermint.com>
Date:   Fri May 1 01:45:01 2020 +0200

    skip tests if builddir is empty

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 92ffad089..2621695fb 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -35,7 +35,7 @@ jobs:
         if: "env.GIT_DIFF != ''"
       - name: test & coverage report creation
         run: |
-          go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock cli_test'
+          go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock cli_test' -builddir=./build/
         if: "env.GIT_DIFF != ''"
       - name: filter out DONTCOVER
         run: |
diff --git a/Makefile b/Makefile
index 0e0876afb..7bdb9ae0f 100644
--- a/Makefile
+++ b/Makefile
@@ -169,8 +169,8 @@ test-sim-benchmark-invariants:
 	-Period=1 -Commit=true -Seed=57 -v -timeout 24h
 
 cli-test: build-sim
-	@go test -mod=readonly -p 4 `go list ./tests/cli/...` -tags=cli_test -v
-	@go test -mod=readonly -p 4 `go list ./x/.../client/cli_test/...` -tags=cli_test -v
+	go test -mod=readonly -p 4 `go list ./tests/cli/...` -tags=cli_test -v -builddir=$(BUILDDIR)
+	go test -mod=readonly -p 4 `go list ./x/.../client/cli_test/...` -tags=cli_test -v -builddir=$(BUILDDIR)
 
 .PHONY: \
 test-sim-nondeterminism \
diff --git a/tests/cli/fixtures.go b/tests/cli/fixtures.go
index 5df955a72..4fb8e74f6 100644
--- a/tests/cli/fixtures.go
+++ b/tests/cli/fixtures.go
@@ -1,8 +1,8 @@
 package cli
 
 import (
+	"flag"
 	"io/ioutil"
-	"os"
 	"path/filepath"
 	"testing"
 
@@ -16,7 +16,10 @@ import (
 	"github.com/cosmos/cosmos-sdk/simapp"
 )
 
-var cdc = std.MakeCodec(simapp.ModuleBasics)
+var (
+	cdc      = std.MakeCodec(simapp.ModuleBasics)
+	buildDir = flag.String("builddir", "", "Build directory")
+)
 
 // Fixtures is used to setup the testing environment
 type Fixtures struct {
@@ -45,15 +48,16 @@ func NewFixtures(t *testing.T) *Fixtures {
 	p2pAddr, _, err := server.FreeTCPAddr()
 	require.NoError(t, err)
 
-	buildDir := os.Getenv("BUILDDIR")
-	require.NotNil(t, buildDir)
+	if *buildDir == "" {
+		t.Skip("builddir is empty, skipping")
+	}
 
 	return &Fixtures{
 		T:            t,
-		BuildDir:     buildDir,
+		BuildDir:     *buildDir,
 		RootDir:      tmpDir,
-		SimdBinary:   filepath.Join(buildDir, "simd"),
-		SimcliBinary: filepath.Join(buildDir, "simcli"),
+		SimdBinary:   filepath.Join(*buildDir, "simd"),
+		SimcliBinary: filepath.Join(*buildDir, "simcli"),
 		SimdHome:     filepath.Join(tmpDir, ".simd"),
 		SimcliHome:   filepath.Join(tmpDir, ".simcli"),
 		RPCAddr:      servAddr,

- name: test & coverage report creation
run: |
go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock'
go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock cli_test'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock cli_test'
go test ./... -mod=readonly -timeout 12m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock cli_test' -builddir=`pwd`/build

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 1, 2020

Codecov Report

Merging #6095 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6095   +/-   ##
=======================================
  Coverage   54.63%   54.63%           
=======================================
  Files         443      443           
  Lines       26666    26666           
=======================================
  Hits        14568    14568           
  Misses      11101    11101           
  Partials      997      997           

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Good to go

@mergify mergify bot merged commit f92f6c9 into cosmos:master May 1, 2020
@sahith-narahari sahith-narahari deleted the sahith/add-simd-tests branch May 1, 2020 20:21
@clevinson clevinson added this to the v0.39 milestone Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:CLI C:x/distribution distribution module related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants