[WIP] Write tests for issue API #86

andreynering · 2016-11-05T17:05:16Z

Please don't merge yet. Opened for discussion on testing approach.

For context see #63

Edit: to run these tests:

go test -v -tags sqlite ./routers/api/v1/repo/

makhov · 2016-11-05T17:17:43Z

modules/setting/setting.go

-	// Does not check run user when the install lock is off.
-	if InstallLock {
+	// Does not check run user when the install lock is off or is running tests
+	if InstallLock && AppRunMode != RUN_MODE_TEST {


Change code for testing is a bad practice

I agree. This is a WIP, we should abstract things using interfaces. I am open for suggestions on how to do it right.

Ok, thank you. It's very important to write testable code 👍

makhov · 2016-11-05T17:20:40Z

models/models.go

@@ -57,6 +57,10 @@ var (
 	EnableTiDB    bool
 )

+func Database() *sql.DB {


And now we can execute SQL queries from handlers?

makhov · 2016-11-05T17:22:53Z

routers/api/v1/repo/issue_test.go

+
+			w, r := testutil.NewTestContext("GET", "/api/v1/repos/user1/foo/issues", "", nil, "1")
+			testutil.ServeHTTP(w, r)
+			So(w.Code, ShouldEqual, http.StatusOK)


Test which test nothing

Well, it tests if /api/v1/repos/user1/foo/issues returns status 200.

I see, but it useless info. /api/v1/repos/user1/foo/issues may returns 200 and {"foo": "bar"} in response body. Or {"oops": "something went wrong"}. Or Fuck the json, etc…

tboerger · 2016-11-05T17:29:05Z

@makhov glad that you are back :)

bkcsoft · 2016-11-14T19:27:25Z

modules/setting/setting.go

@@ -263,6 +269,10 @@ var (
 	HasRobotsTxt bool
 )

+func GiteaPath() string {


Exported functions should have be commented 😄

bkcsoft · 2016-11-14T19:29:16Z

testdata/app_test.ini

@@ -0,0 +1,31 @@
+# THIS FILE EXISTS TO BE USED TO UNIT TESTS
+# DON'T CHANGE IT. THIS IS NOT THE FILE GOGS WILL USE IN PRODUCTION


bkcsoft · 2016-11-14T19:29:38Z

testdata/app_test.ini

+# THIS FILE EXISTS TO BE USED TO UNIT TESTS
+# DON'T CHANGE IT. THIS IS NOT THE FILE GOGS WILL USE IN PRODUCTION
+
+APP_NAME = Gogs: Go Git Service


Same as above

bkcsoft · 2016-12-25T23:30:01Z

As I'm a big promoter of unit-tests, I'd prefer if we'd start with the model-tests instead. That way we get rid of a bunch of edge-cases before going for e2e-tests (integration, like these ones). It's also extremely hard to force edge-cases from this level, and the amount of tests would blow out of proportions. Testing models is easier since we know what goes in an out, and we don't have to force edge-cases. I also suggest that we use in-memory sqlite database

andreynering · 2016-12-27T21:40:05Z

So this need more discussion...

xinity added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Nov 5, 2016

andreynering mentioned this pull request Nov 5, 2016

Testability Strategy #63

Closed

makhov suggested changes Nov 5, 2016

View reviewed changes

tboerger added this to the 1.0.0 milestone Nov 5, 2016

DblK added the pr/wip This PR is not ready for review label Nov 5, 2016

strk removed this from the 1.0.0 milestone Nov 6, 2016

andreynering added the type/testing label Nov 6, 2016

tboerger added this to the 1.0.0 milestone Nov 7, 2016

andreynering added 2 commits November 8, 2016 21:16

Vendor gopkg.in/testfixtures.v2

700831a

Write tests for Issue API

777ff65

bkcsoft suggested changes Nov 14, 2016

View reviewed changes

lunny modified the milestones: 1.1.0, 1.0.0 Nov 17, 2016

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

andreynering closed this Dec 27, 2016

ethantkoenig mentioned this pull request Dec 29, 2016

Unit tests for token and update models #526

Merged

tboerger added reviewed/invalid and removed type/proposal The new feature has not been accepted yet but needs to be discussed first. type/testing lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review labels Jan 16, 2017

tboerger removed this from the 1.1.0 milestone Jan 16, 2017

lunny pushed a commit to lunny/gitea that referenced this pull request Feb 7, 2019

Update MAINTAINERS (go-gitea#86)

cdf53e0

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

delvh added issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea and removed reviewed/invalid labels Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Write tests for issue API #86

[WIP] Write tests for issue API #86

andreynering commented Nov 5, 2016 •

edited

Loading

makhov Nov 5, 2016

andreynering Nov 5, 2016

makhov Nov 6, 2016

makhov Nov 5, 2016

makhov Nov 5, 2016

andreynering Nov 5, 2016

makhov Nov 6, 2016

tboerger commented Nov 5, 2016

bkcsoft Nov 14, 2016

bkcsoft Nov 14, 2016

bkcsoft Nov 14, 2016

bkcsoft commented Dec 25, 2016

andreynering commented Dec 27, 2016

		@@ -0,0 +1,31 @@
		# THIS FILE EXISTS TO BE USED TO UNIT TESTS
		# DON'T CHANGE IT. THIS IS NOT THE FILE GOGS WILL USE IN PRODUCTION

[WIP] Write tests for issue API #86

[WIP] Write tests for issue API #86

Conversation

andreynering commented Nov 5, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tboerger commented Nov 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkcsoft commented Dec 25, 2016

andreynering commented Dec 27, 2016

andreynering commented Nov 5, 2016 •

edited

Loading