From c15eb72b0649d737e517f5dae4688204e5f5d123 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Tue, 14 May 2019 14:51:57 +0200 Subject: [PATCH] Cleanup testing procedures - remove "unit" build flag - make sure tests without build flag (integration, external) run without deps - move unit-tests step in front of "docker image build" drone pipeline --- .drone.yml | 53 ++++++------------- Makefile | 15 ++++-- compose/internal/repository/module_test.go | 2 +- .../internal/repository/ql/ast_parser_test.go | 2 - compose/internal/repository/ql/lexer_test.go | 2 - .../repository/record_report_builder_test.go | 2 - compose/internal/repository/record_test.go | 2 - .../internal/repository/repository_test.go | 2 +- internal/mail/mail_test.go | 2 - internal/store/store_test.go | 2 - messaging/internal/repository/events_test.go | 2 - .../internal/repository/pubsub_memory_test.go | 2 - .../internal/repository/repository_test.go | 2 +- messaging/internal/service/channel.go | 36 ++++++++----- messaging/internal/service/channel_test.go | 12 ++--- messaging/internal/service/message.go | 5 +- messaging/internal/service/message_test.go | 12 ++--- messaging/types/mention_test.go | 2 - system/internal/repository/repository_test.go | 2 +- 19 files changed, 70 insertions(+), 89 deletions(-) diff --git a/.drone.yml b/.drone.yml index 403577ec3..fa9b3c591 100644 --- a/.drone.yml +++ b/.drone.yml @@ -6,23 +6,20 @@ kind: pipeline name: docker image build steps: -#- name: build -# image: crusttech/crust-builder:latest -# pull: always -# environment: -# CGO_ENABLED: 0 -# GOOS: linux -# GOARCH: amd64 -# commands: -# - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/crust-$GOOS-$GOARCH cmd/crust/*.go -# - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/compose-$GOOS-$GOARCH cmd/compose/*.go -# - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/messaging-$GOOS-$GOARCH cmd/messaging/*.go -# - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/system-$GOOS-$GOARCH cmd/system/*.go -# - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/system-cli-$GOOS-$GOARCH cmd/system-cli/*.go +- name: unit-tests + image: crusttech/crust-builder:latest + pull: always + environment: + CGO_ENABLED: 0 + GOOS: linux + GOARCH: amd64 + CI: circleci + commands: + - make test - name: api-system image: plugins/docker - depends_on: [ clone ] + depends_on: [ 'unit-tests' ] settings: repo: crusttech/api-system auto_tag: true # generate tag names automatically based on git branch and git tag @@ -35,7 +32,7 @@ steps: - name: api-compose image: plugins/docker - depends_on: [ clone ] + depends_on: [ 'unit-tests' ] settings: repo: crusttech/api-compose auto_tag: true # generate tag names automatically based on git branch and git tag @@ -46,7 +43,7 @@ steps: from_secret: docker_hub_password - name: api-messaging - depends_on: [ clone ] + depends_on: [ 'unit-tests' ] image: plugins/docker settings: repo: crusttech/api-messaging @@ -60,7 +57,7 @@ steps: - name: crust image: plugins/docker - depends_on: [ clone ] + depends_on: [ 'unit-tests' ] settings: repo: crusttech/crust auto_tag: true # generate tag names automatically based on git branch and git tag @@ -102,15 +99,12 @@ steps: COMPOSE_DB_DSN: crust:crust@tcp(crust-db:3306)/crust?collation=utf8mb4_general_ci MESSAGING_DB_DSN: crust:crust@tcp(crust-db:3306)/crust?collation=utf8mb4_general_ci commands: - # Run tests without any dependencies - - go test ./cmd/... ./internal/... ./compose/... ./messaging/... ./system/... - # Wait for the db before running other tests - wait-for-it.sh -t 60 --strict crust-db:3306 -- echo "Crust DB1 is up" - gotest -v --tags="migrations" ./system/db/... - gotest -v --tags="migrations" ./compose/db/... - gotest -v --tags="migrations" ./messaging/db/... - - go test -run=^$ --tags="unit integration external" ./cmd/... ./internal/... ./compose/... ./messaging/... ./system/... - - gotest -failfast --coverprofile=coverage.txt -v --tags="unit integration" ./internal/... ./system/... ./compose/... ./messaging/... + - go test -run=^$ --tags="integration external" ./cmd/... ./internal/... ./compose/... ./messaging/... ./system/... + - gotest -failfast --coverprofile=coverage.txt -v --tags="integration" ./internal/... ./system/... ./compose/... ./messaging/... - name: coverage image: plugins/codecov @@ -119,21 +113,6 @@ steps: files: - coverage.txt -- name: build - image: crusttech/crust-builder:latest - pull: always - environment: - CGO_ENABLED: 0 - GOOS: linux - GOARCH: amd64 - commands: - - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/crust-$GOOS-$GOARCH cmd/crust/*.go - - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/compose-$GOOS-$GOARCH cmd/compose/*.go - - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/messaging-$GOOS-$GOARCH cmd/messaging/*.go - - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/system-$GOOS-$GOARCH cmd/system/*.go - - go build -ldflags "-X github.com/crusttech/crust/internal/version.BuildTime=`date +%FT%T%z` -X github.com/crusttech/crust/internal/version.Version=`git describe --always --tags`" -o build/system-cli-$GOOS-$GOARCH cmd/system-cli/*.go - - services: - name: crust-db pull: always diff --git a/Makefile b/Makefile index 11979bb7b..e63e26b7a 100644 --- a/Makefile +++ b/Makefile @@ -64,6 +64,10 @@ mailhog.up: ######################################################################################################################## # QA +test: + # Run basic unit tests + $(GO) test ./cmd/... ./internal/... ./compose/... ./messaging/... ./system/... + test.internal: $(GOTEST) $(GOTEST) -covermode count -coverprofile .cover.out -v ./internal/... $(GO) tool cover -func=.cover.out @@ -104,6 +108,12 @@ test.cross-dep: grep -rE "crust/(system|messaging)/" compose || exit 0 grep -rE "crust/(system|compose)/" messaging || exit 0 +integration: + # Run drone's integration pipeline + rm -f build/gen* + drone exec --pipeline integration + + vet: $(GO) vet ./... @@ -145,8 +155,3 @@ $(MOCKGEN): clean: rm -f $(REALIZE) $(GOCRITIC) $(GOTEST) - - -integration: - rm -f build/gen* - drone exec --pipeline integration diff --git a/compose/internal/repository/module_test.go b/compose/internal/repository/module_test.go index b80cc24a7..4649965ab 100644 --- a/compose/internal/repository/module_test.go +++ b/compose/internal/repository/module_test.go @@ -1,4 +1,4 @@ -// +build unit integration +// +build integration package repository diff --git a/compose/internal/repository/ql/ast_parser_test.go b/compose/internal/repository/ql/ast_parser_test.go index 7705b230f..2ec478598 100644 --- a/compose/internal/repository/ql/ast_parser_test.go +++ b/compose/internal/repository/ql/ast_parser_test.go @@ -1,5 +1,3 @@ -// +build unit - package ql import ( diff --git a/compose/internal/repository/ql/lexer_test.go b/compose/internal/repository/ql/lexer_test.go index f732e3f3d..e7f1633e5 100644 --- a/compose/internal/repository/ql/lexer_test.go +++ b/compose/internal/repository/ql/lexer_test.go @@ -1,5 +1,3 @@ -// +build unit - package ql import ( diff --git a/compose/internal/repository/record_report_builder_test.go b/compose/internal/repository/record_report_builder_test.go index 0c3008aad..c71fa2958 100644 --- a/compose/internal/repository/record_report_builder_test.go +++ b/compose/internal/repository/record_report_builder_test.go @@ -1,5 +1,3 @@ -// +build unit - package repository import ( diff --git a/compose/internal/repository/record_test.go b/compose/internal/repository/record_test.go index 12107aa01..611686a53 100644 --- a/compose/internal/repository/record_test.go +++ b/compose/internal/repository/record_test.go @@ -1,5 +1,3 @@ -// +build unit - package repository /* diff --git a/compose/internal/repository/repository_test.go b/compose/internal/repository/repository_test.go index 03dc39bc5..a2b20f8ca 100644 --- a/compose/internal/repository/repository_test.go +++ b/compose/internal/repository/repository_test.go @@ -1,4 +1,4 @@ -// +build unit integration +// +build integration package repository diff --git a/internal/mail/mail_test.go b/internal/mail/mail_test.go index 70bb55a36..857cf351b 100644 --- a/internal/mail/mail_test.go +++ b/internal/mail/mail_test.go @@ -1,5 +1,3 @@ -// +build unit - package mail import ( diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 61d270440..a80266f18 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -1,5 +1,3 @@ -// +build unit - package store import ( diff --git a/messaging/internal/repository/events_test.go b/messaging/internal/repository/events_test.go index 7f16ab9c5..d1bb53510 100644 --- a/messaging/internal/repository/events_test.go +++ b/messaging/internal/repository/events_test.go @@ -1,5 +1,3 @@ -// +build unit - package repository import ( diff --git a/messaging/internal/repository/pubsub_memory_test.go b/messaging/internal/repository/pubsub_memory_test.go index 1ef2c754e..d5f6667f4 100644 --- a/messaging/internal/repository/pubsub_memory_test.go +++ b/messaging/internal/repository/pubsub_memory_test.go @@ -1,5 +1,3 @@ -// +build unit - package repository import ( diff --git a/messaging/internal/repository/repository_test.go b/messaging/internal/repository/repository_test.go index db57c0852..52ad121c8 100644 --- a/messaging/internal/repository/repository_test.go +++ b/messaging/internal/repository/repository_test.go @@ -1,4 +1,4 @@ -// +build unit integration +// +build integration package repository diff --git a/messaging/internal/service/channel.go b/messaging/internal/service/channel.go index f53611f5a..44f3d1312 100644 --- a/messaging/internal/service/channel.go +++ b/messaging/internal/service/channel.go @@ -208,6 +208,18 @@ func (svc *channel) FindMembers(channelID uint64) (out types.ChannelMemberSet, e } func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { + if len(in.Name) == 0 && in.Type != types.ChannelTypeGroup { + return nil, errors.New("channel name not provided") + } + + if settingsChannelNameLength > 0 && len(in.Name) > settingsChannelNameLength { + return nil, errors.Errorf("channel name (%d characters) too long (max: %d)", len(in.Name), settingsChannelNameLength) + } + + if len(in.Topic) > 0 && settingsChannelTopicLength > 0 && len(in.Topic) > settingsChannelTopicLength { + return nil, errors.Errorf("channel topic (%d characters) too long (max: %d)", len(in.Topic), settingsChannelTopicLength) + } + return out, svc.db.Transaction(func() (err error) { var msg *types.Message @@ -240,18 +252,6 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { return errors.WithStack(ErrNoPermissions) } - if len(in.Name) == 0 && in.Type != types.ChannelTypeGroup { - return errors.New("channel name not provided") - } - - if settingsChannelNameLength > 0 && len(in.Name) > settingsChannelNameLength { - return errors.Errorf("channel name (%d characters) too long (max: %d)", len(in.Name), settingsChannelNameLength) - } - - if len(in.Topic) > 0 && settingsChannelTopicLength > 0 && len(in.Topic) > settingsChannelTopicLength { - return errors.Errorf("channel topic (%d characters) too long (max: %d)", len(in.Topic), settingsChannelTopicLength) - } - // This is a fresh channel, just copy values out = &types.Channel{ Name: in.Name, @@ -340,6 +340,18 @@ func (svc *channel) checkGroupExistance(mm types.ChannelMemberSet) (out *types.C } func (svc *channel) Update(in *types.Channel) (ch *types.Channel, err error) { + if len(in.Name) == 0 && in.Type != types.ChannelTypeGroup { + return nil, errors.New("channel name not provided") + } + + if settingsChannelNameLength > 0 && len(in.Name) > settingsChannelNameLength { + return nil, errors.Errorf("channel name (%d characters) too long (max: %d)", len(in.Name), settingsChannelNameLength) + } + + if len(in.Topic) > 0 && settingsChannelTopicLength > 0 && len(in.Topic) > settingsChannelTopicLength { + return nil, errors.Errorf("channel topic (%d characters) too long (max: %d)", len(in.Topic), settingsChannelTopicLength) + } + return ch, svc.db.Transaction(func() (err error) { var changed bool diff --git a/messaging/internal/service/channel_test.go b/messaging/internal/service/channel_test.go index d6450780d..8ebef8159 100644 --- a/messaging/internal/service/channel_test.go +++ b/messaging/internal/service/channel_test.go @@ -1,5 +1,3 @@ -// +build unit - package service import ( @@ -17,11 +15,13 @@ func TestChannelNameTooShort(t *testing.T) { ctx := context.Background() ctx = auth.SetIdentityToContext(ctx, &systemTypes.User{}) - svc := channel{db: &mockDB{}, ctx: ctx} + svc := channel{} e := func(out *types.Channel, err error) error { return err } - longName := strings.Repeat("X", settingsChannelNameLength+1) - test.Assert(t, e(svc.Create(&types.Channel{})) != nil, "Should not allow to create unnamed channels") - test.Assert(t, e(svc.Create(&types.Channel{Name: longName})) != nil, "Should not allow to create channel with really long name") + + if settingsChannelNameLength > 0 { + longName := strings.Repeat("X", settingsChannelNameLength+1) + test.Assert(t, e(svc.Create(&types.Channel{Name: longName})) != nil, "Should not allow to create channel with really long name") + } } diff --git a/messaging/internal/service/message.go b/messaging/internal/service/message.go index f87cb4d23..254db6cf1 100644 --- a/messaging/internal/service/message.go +++ b/messaging/internal/service/message.go @@ -197,11 +197,14 @@ func (svc message) Create(in *types.Message) (message *types.Message, err error) } in.Message = strings.TrimSpace(in.Message) + var mlen = len(in.Message) if mlen == 0 { return nil, errors.Errorf("refusing to create message without contents") - } else if settingsMessageBodyLength > 0 && mlen > settingsMessageBodyLength { + } + + if settingsMessageBodyLength > 0 && mlen > settingsMessageBodyLength { return nil, errors.Errorf("message length (%d characters) too long (max: %d)", mlen, settingsMessageBodyLength) } diff --git a/messaging/internal/service/message_test.go b/messaging/internal/service/message_test.go index 5b5532bf8..dc9dcdfb8 100644 --- a/messaging/internal/service/message_test.go +++ b/messaging/internal/service/message_test.go @@ -1,5 +1,3 @@ -// +build unit - package service import ( @@ -17,13 +15,15 @@ func TestMessageLength(t *testing.T) { ctx := context.Background() ctx = auth.SetIdentityToContext(ctx, &systemTypes.User{}) - svc := message{db: &mockDB{}, ctx: ctx} + svc := message{} e := func(out *types.Message, err error) error { return err } - longText := strings.Repeat("X", settingsMessageBodyLength+1) - test.Assert(t, e(svc.Create(&types.Message{})) != nil, "Should not allow to create empty message") - test.Assert(t, e(svc.Create(&types.Message{Message: longText})) != nil, "Should not allow to create message with really long text") + + if settingsMessageBodyLength > 0 { + longText := strings.Repeat("X", settingsMessageBodyLength+1) + test.Assert(t, e(svc.Create(&types.Message{Message: longText})) != nil, "Should not allow to create message with really long text") + } } func TestMentionsExtraction(t *testing.T) { diff --git a/messaging/types/mention_test.go b/messaging/types/mention_test.go index d1c5f6e1c..cf9e2b9bc 100644 --- a/messaging/types/mention_test.go +++ b/messaging/types/mention_test.go @@ -1,5 +1,3 @@ -// +build unit - package types import ( diff --git a/system/internal/repository/repository_test.go b/system/internal/repository/repository_test.go index db57c0852..52ad121c8 100644 --- a/system/internal/repository/repository_test.go +++ b/system/internal/repository/repository_test.go @@ -1,4 +1,4 @@ -// +build unit integration +// +build integration package repository