Unit tests for token and update models #526

ethantkoenig · 2016-12-29T01:07:19Z

Taking a cue from @bkcsoft's comment (#86 (comment)), this PR adds unit tests for model/token.go and models/update.go using an in-memory SQLite database.

lunny · 2016-12-29T01:29:33Z

models/tests.go

+
+func insert(insertBeans ...interface{}) error {
+	sess := x.NewSession()
+	_, err := sess.Insert(insertBeans...)


defer sess.Close() MUST add this.

lunny · 2016-12-29T01:29:43Z

models/tests.go

+
+func delete(deleteBeans ...interface{}) error {
+	sess := x.NewSession()
+	// non-zero limit allows for condition-less deletes. Since it is very


defer sess.Close() MUST add this.

lunny · 2016-12-29T01:30:08Z

models/update_test.go

+	suite.NoError(AddUpdateTask(task))
+
+	sess := x.NewSession()
+	has, err := sess.Get(task)


lunny · 2016-12-29T01:30:17Z

models/update_test.go

+	suite.NoError(PopulateUpdateTaskData())
+	suite.NoError(DeleteUpdateTaskByUUID("uuid1"))
+	sess := x.NewSession()
+	has, err := sess.Get(&UpdateTask{UUID: "uuid1"})


ethantkoenig · 2016-12-29T01:38:00Z

@lunny Thanks for the quick feedback, I've added defer sess.Close() calls

lunny · 2016-12-29T08:28:51Z

models/tests.go

+import (
+	"github.com/go-xorm/core"
+	"github.com/go-xorm/xorm"
+	_ "github.com/mattn/go-sqlite3" // for the test engine


I would like you use tidb as test db since go-sqlite3 will ask cgo.

In the future we can use build tags to allow choose what DB we will run test agains: MySQL/PG/SQLite/etc

testfixtures does not support tidb, so we won't be able to use both testfixtures and tidb. Should I switch to tidb and continue manually deleting/inserting beans, or keep using sqlite and use testfixtures?

(By the way, what does CGO stand for?)

I'd keep SQLite for now, until I can add support to TiDB on testfixtures.

@lunny What do you think?

andreynering

Thank you @ethantkoenig

I think this approach is very close to what the team had in mind.

/cc @bkcsoft

andreynering · 2016-12-29T13:19:41Z

models/tests.go

+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models


I think this file should be named something_test.go, so it whould not be compiled if not running tests.

Renamed to setup_for_test.go.

andreynering · 2016-12-29T13:21:03Z

models/tests.go

+import (
+	"github.com/go-xorm/core"
+	"github.com/go-xorm/xorm"
+	_ "github.com/mattn/go-sqlite3" // for the test engine


In the future we can use build tags to allow choose what DB we will run test agains: MySQL/PG/SQLite/etc

andreynering · 2016-12-29T13:22:14Z

models/tests.go

+	return nil
+}
+
+func deleteTestBeans(deleteBeans... interface{}) error {


Should we use testfixtures instead of manually deleting/inserting beans?

The biggest problem of doing that manually is having a lot of foreign key violations while running tests.

andreynering · 2016-12-29T13:23:25Z

models/token_test.go

+import (
+	"github.com/stretchr/testify/suite"
+	"testing"
+)


Import grouping:

import ( "testing" "github.com/stretchr/testify/suite" )

andreynering · 2016-12-29T13:24:57Z

models/update_test.go

+	"github.com/stretchr/testify/suite"
+	"testing"
+	"time"
+)


import ( "container/list" "testing" "time" "code.gitea.io/git" "github.com/stretchr/testify/suite" )

andreynering · 2016-12-29T13:27:05Z

models/token_test.go

+
+type TokenTestSuite struct {
+	suite.Suite
+}


Should we use a suite or just call assert functions?

I used test suites because they can ensure that CreateTestEngine() is called before any tests run. If there is some way to ensure this without suites, I'd be open to changing.

I'm fine with both approaches, but I think this would work, too:

func TestMain(m *testing.M) { // create engine here }

But if we have multiple test files, won't there be a name collision if they each have a TestMain?

@ethantkoenig You're right, we can have only one per package. If that is not enough I'm fine with keeping the suite, but isn't it enough to init the engine once?

Fair enough, updated.

bkcsoft

Also I'd suggest using https://github.com/go-testfixtures/testfixtures instead of hard-coded data :)

bkcsoft · 2016-12-29T19:58:35Z

models/setup_for_test.go

+}
+
+// CreateTestEngine create an xorm engine for testing
+func CreateTestEngine() error {


Not a huge fan of using custom db-setups for this, though seeing as models.LoadConfigs requires the use of app.ini it can't easily be done w/o that...

andreynering · 2016-12-30T18:42:48Z

LGTM

bkcsoft · 2016-12-30T19:44:04Z

LGTM 🎉

bkcsoft

Missed this one

bkcsoft · 2016-12-30T19:46:09Z

vendor/vendor.json

+		},
+		{
+			"checksumSHA1": "Dp8TnT65nINpRF/PrxrgmnYLsyA=",
+			"path": "github.com/stretchr/testify/suite",


Is this still in use? otherwise please remove it :)

ethantkoenig · 2016-12-30T20:22:33Z

@bkcsoft Whoops, removed

ethantkoenig changed the title ~~Unit tests token and update models~~ Unit tests for token and update models Dec 29, 2016

ethantkoenig force-pushed the models/unittests branch 2 times, most recently from 37a945d to 709a336 Compare December 29, 2016 01:27

lunny added this to the 1.1.0 milestone Dec 29, 2016

lunny added type/enhancement An improvement of existing functionality type/testing labels Dec 29, 2016

lunny requested changes Dec 29, 2016

View reviewed changes

ethantkoenig force-pushed the models/unittests branch from 709a336 to 8e151cd Compare December 29, 2016 01:36

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 29, 2016

ethantkoenig force-pushed the models/unittests branch 2 times, most recently from c6ee02b to fd075c0 Compare December 29, 2016 02:29

lunny approved these changes Dec 29, 2016

View reviewed changes

lunny reviewed Dec 29, 2016

View reviewed changes

andreynering reviewed Dec 29, 2016

View reviewed changes

bkcsoft reviewed Dec 29, 2016

View reviewed changes

bkcsoft added the pr/wip This PR is not ready for review label Dec 29, 2016

ethantkoenig force-pushed the models/unittests branch 2 times, most recently from c8924c8 to d076fe6 Compare December 30, 2016 18:31

ethantkoenig changed the title ~~Unit tests for token and update models~~ [WIP] Unit tests for token and update models Dec 30, 2016

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 30, 2016

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 30, 2016

bkcsoft suggested changes Dec 30, 2016

View reviewed changes

Unit tests for token and update models

12ba4e1

ethantkoenig force-pushed the models/unittests branch from d076fe6 to 12ba4e1 Compare December 30, 2016 20:22

ethantkoenig changed the title ~~[WIP] Unit tests for token and update models~~ Unit tests for token and update models Dec 30, 2016

andreynering removed the pr/wip This PR is not ready for review label Dec 30, 2016

bkcsoft approved these changes Dec 31, 2016

View reviewed changes

bkcsoft merged commit de8b73d into go-gitea:master Dec 31, 2016

ethantkoenig deleted the models/unittests branch December 31, 2016 16:38

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit tests for token and update models #526

Unit tests for token and update models #526

ethantkoenig commented Dec 29, 2016

lunny Dec 29, 2016

lunny Dec 29, 2016

lunny Dec 29, 2016

lunny Dec 29, 2016

ethantkoenig commented Dec 29, 2016 •

edited

Loading

lunny Dec 29, 2016

andreynering Dec 29, 2016

ethantkoenig Dec 29, 2016 •

edited

Loading

andreynering Dec 29, 2016

lunny Dec 30, 2016

andreynering left a comment

andreynering Dec 29, 2016

lunny Dec 29, 2016

ethantkoenig Dec 29, 2016

andreynering Dec 29, 2016

andreynering Dec 29, 2016

andreynering Dec 29, 2016

ethantkoenig Dec 29, 2016

andreynering Dec 29, 2016

ethantkoenig Dec 29, 2016

andreynering Dec 29, 2016

ethantkoenig Dec 29, 2016

andreynering Dec 29, 2016 •

edited

Loading

ethantkoenig Dec 29, 2016

andreynering Dec 29, 2016

ethantkoenig Dec 29, 2016

bkcsoft left a comment

bkcsoft Dec 29, 2016

andreynering commented Dec 30, 2016

bkcsoft commented Dec 30, 2016

bkcsoft left a comment

bkcsoft Dec 30, 2016

ethantkoenig commented Dec 30, 2016

Unit tests for token and update models #526

Unit tests for token and update models #526

Conversation

ethantkoenig commented Dec 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethantkoenig commented Dec 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethantkoenig Dec 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreynering left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreynering Dec 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkcsoft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreynering commented Dec 30, 2016

bkcsoft commented Dec 30, 2016

bkcsoft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethantkoenig commented Dec 30, 2016

ethantkoenig commented Dec 29, 2016 •

edited

Loading

ethantkoenig Dec 29, 2016 •

edited

Loading

andreynering Dec 29, 2016 •

edited

Loading