Integration test framework #1290

ethantkoenig · 2017-03-16T20:07:42Z

NOTE: Drone is currently failing due to .drone.yml.sig, which needs to be updated.

A new framework for integration tests. The main issues with our current integration tests are that it

Uses the production database
Re-installs the server for every test
Must manually add content (e.g. users, repos) to the system for every test

My proposal is to have a test "environment" (database and filesystem containing repos). The integration test environment would be populated with "fixtures", somewhat similar to what we do for unit tests (plus actual repos in the filesystem). Right now, this is implemented by having a separate app.ini for integration tests, as well as a collection of database fixtures and directory of repositories.

Also implements @andreynering's suggestion from #1135 (comment) of using the ServeHTTP(..) method

This system would allow us to not only add integration tests, but also unit tests for components that interact with the filesystem (e.g. anything that runs git commands).

STORING THE INTEGRATION ENVIRONMENT:
In order to not have to store a bunch of test fixtures and repos inside the main gitea repository, I have created a separate repo (https://github.com/ethantkoenig/gitea-integration) that stores the integration test fixtures and repositories. This repo must be cloned to run integration tests. If we end up choosing this approach, it might make sense to transfer this repo to the go-gitea organization.

WHAT TO DO WITH EXISTING INTEGRATION TESTS:
For now, I have removed the existing integration tests to clear up the integrations/ directory. I have ported all previous integration tests to the new framework.

HOW TO RUN INTEGRATION TESTS:

make test-(sqlite|mysql|pgsql). You may need to update the corresponding integrations/*.ini to match whatever you have running locally.

bkcsoft · 2017-03-16T21:19:24Z

cmd/web.go

@@ -71,7 +71,7 @@ and it takes care of all the other things for you`,
 }

 // newMacaron initializes Macaron instance.
-func newMacaron() *macaron.Macaron {
+func NewMacaron() *macaron.Macaron {


Can we move this (and RegisterRoutes to a new modules/router instead? Since they really shouldn't be here anyway 🙂 )

bkcsoft · 2017-03-16T21:20:04Z

models/models.go

@@ -24,6 +24,9 @@ import (
 	// Needed for the MSSSQL driver
 	_ "github.com/denisenkom/go-mssqldb"

+	// Needed for SQLite
+	_ "github.com/mattn/go-sqlite3"


nononononono, there's a reason that this isn't here...

I know, I added it as a temporary hack so the tests would work; I realize that sqlite has to be explicitly enabled. Perhaps I should have made that clearer.

I figured out the non-hacky way to make things work, so this has been removed.

bkcsoft · 2017-03-16T21:21:19Z

myintegrations/gitea-repositories/user1/repo1.git/hooks/post-receive.d/gitea

@@ -0,0 +1,2 @@
+#!/usr/bin/env bash
+"/home/ethantkoenig/.gopath/src/code.gitea.io/gitea/gitea" hook --config='integrations/app.ini' post-receive


This will break for everyone... Don't put the test-repos in here, have the test pull it from somewhere (or untar/unzip)

Hmm, I didn't realize there were hard-coded absolute paths in here. If we pull from somewhere else, we'll still have to edit all of these scripts to have the correct path (which is doable, but annoying).

I've updated the scripts to use $GITEA_ROOT instead of a hard-coded path. We'll have to export the appropriate value for GITEA_ROOT before running integration tests involving these scripts.

bkcsoft · 2017-03-16T21:24:44Z

Also don't call it myintegrations, just go for intergrations or tests/integration :)

ethantkoenig · 2017-03-17T00:26:29Z

Rebased, renamed myintegrations to integrations and remove the previous contents of integrations.

lunny · 2017-03-17T05:09:35Z

It seems #741 had imported the integration tests and #1135 added signup tests.

strk · 2017-03-17T08:41:35Z

I'd like the integration test framework to also let me test PostgreSQL as a backend (for example). So I'd prefer fixtures to be passed to the ORM to fill the target DB of choice for testing.

ethantkoenig · 2017-03-17T14:12:02Z

@lunny I am aware, but there are some problems with these integration tests (see PR description). It was my understanding that #741 and #1135 were not a permanent solution (#1135 (comment)).

ethantkoenig · 2017-03-20T01:10:58Z

Rebased to resolve conflicts.

@strk I've updated it to use testfixtures, so to use another RDBMS all you have to do is modify integrations/app.ini

strk · 2017-03-20T09:15:55Z

@ethantkoenig would it be possible to script the app.ini update so to have make check (or go test if possible) run the test against all supported (at build-time) database backends ?

ethantkoenig · 2017-03-23T21:06:05Z

@strk I've created separate .ini files for SQLite, MySQL, and PostgreSQL. The integration tests can be run using make test-(sqlite|mysql|pgsql). All three pass locally, but I've been having some trouble getting MySQL and PostgreSQL to pass in CI.

strk · 2017-03-23T22:48:19Z

.gitignore

@@ -44,3 +44,5 @@ coverage.out
 /indexers
 /log
 /public/img/avatar
+


empty line ?

strk · 2017-03-23T22:49:25Z

.drone.yml

@@ -42,7 +53,7 @@ pipeline:
      TAGS: bindata
      GOPATH: /srv/app
    commands:
-      - make test-pgsql
+      - echo 'Not ready yet'


Maybe you could leave the intended command in here, for reference, like echo make 'test-pgsql #not ready yet' (and same for all non-ready blocks)

strk · 2017-03-23T22:49:41Z

.drone.yml

+      TAGS: bindata
+      GOPATH: /srv/app
+    commands:
+      - echo 'Not ready yet'


I thought you mentioned this was actually passing on drone ?

strk

Please don't add that tar.gz to the repository (what is it for?).

Also, I don't understand the changes to cmd/web.go

ethantkoenig · 2017-03-23T23:20:20Z

The .tar.gz was accidentally added; I've removed it, and added it to .gitignore.

The changes to cmd/web.go are following @bkcsoft's suggestion above

andreynering · 2017-03-23T23:41:31Z

Drone error seems unrelated to the change:

WARNING: unable to verify the Yaml signature.

andreynering · 2017-03-23T23:47:25Z

LGTM 🎉

I'd just disable Xorm log output on console.

strk · 2017-04-20T15:17:33Z

@bkcsoft why modules/router/router.go rather than routers/something ?

lunny · 2017-04-21T05:47:18Z

Makefile

+	go test -c code.gitea.io/gitea/integrations -tags 'sqlite'
+
+integrations/gitea-integration:
+	curl -L https://github.com/ethantkoenig/gitea-integration/archive/v2.tar.gz > integrations/gitea-integration.tar.gz


What's the archive?

It contains the git repositories for the integration tests. It is necessary, because some integration may need to run git commands, or access files in the repositories.

See also the "STORING THE INTEGRATION ENVIRONMENT" section of the PR description.

lunny · 2017-04-21T05:47:40Z

integrations/integration_test.go

+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/routers"
+
+	"bytes"


wrong import place

lunny · 2017-04-21T05:49:15Z

integrations/signup_test.go

-	"code.gitea.io/gitea/integrations/internal/utils"
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/setting"
+	"github.com/stretchr/testify/assert"


blank line.

lunny · 2017-04-21T05:50:28Z

integrations/view_test.go

+	"testing"
+
+	"code.gitea.io/gitea/models"
+	"github.com/stretchr/testify/assert"


bkcsoft · 2017-04-21T07:29:27Z

@strk @ethantkoenig I didn't really have a reason for modules/routes/ over routers/, feel free to move it. But it didn't belong as an exported function in cmd/web.go (nothing should import /cmd except for main.go 😉 )

ethantkoenig · 2017-04-23T13:32:46Z

@lunny I've addressed your review, and moved modules/router/router.go to routers

lunny · 2017-04-24T01:29:59Z

LGTM

appleboy · 2017-04-24T01:52:53Z

.drone.yml

  test-mysql:
    image: webhippie/golang:edge
    pull: true
    environment:
      TAGS: bindata
      GOPATH: /srv/app
    commands:
-      - make test-mysql
+      - echo make test-mysql # Not ready yet


revert this changes? @ethantkoenig

test-mysql and test-pgsql still aren't ready to run in CI (although they do pass locally). Unfortunately, I've had a hard time debugging why they aren't passing in CI, and given how long it took to get a .drone.yml.sig for this PR, I think makes more sense to address them in a future PR.

See also #1290 (comment)

appleboy · 2017-04-24T01:53:02Z

.drone.yml

@@ -42,7 +53,7 @@ pipeline:
      TAGS: bindata
      GOPATH: /srv/app
    commands:
-      - make test-pgsql
+      - echo make test-pqsql # Not ready yet


revert this changes? @ethantkoenig

ethantkoenig · 2017-04-24T03:20:48Z

@strk I believe I have addressed all of your requested changes.

strk · 2017-04-24T10:19:13Z

Thanks, all checks pass now, just needs to be merged. Looking forward to enable the CI testing of mysql and postgresql !

lunny · 2017-04-24T14:36:30Z

conflicted. @ethantkoenig

ethantkoenig · 2017-04-24T15:39:27Z

@lunny Rebased to resolve conflicts. Unfortunately, I will need a new .drone.yml.sig now that #1468 has been merged into master.

lunny · 2017-04-25T05:59:06Z

@ethantkoenig I have resigned it.

bkcsoft suggested changes Mar 16, 2017

View reviewed changes

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 16, 2017

ethantkoenig force-pushed the test/integration branch from 364f6c7 to 474e231 Compare March 17, 2017 00:24

lunny added the pr/wip This PR is not ready for review label Mar 17, 2017

ethantkoenig force-pushed the test/integration branch 2 times, most recently from e5f0858 to 6ca850a Compare March 20, 2017 01:09

ethantkoenig changed the title ~~[WIP] Integration test framework~~ Integration test framework Mar 20, 2017

ethantkoenig force-pushed the test/integration branch from 4fcb88b to 3541bd1 Compare March 20, 2017 03:00

lunny added this to the 1.2.0 milestone Mar 20, 2017

lunny added the type/testing label Mar 20, 2017

ethantkoenig force-pushed the test/integration branch 2 times, most recently from a267737 to 1eeae42 Compare March 23, 2017 21:05

ethantkoenig force-pushed the test/integration branch 2 times, most recently from 5d08ed1 to 96669d4 Compare March 23, 2017 21:15

strk reviewed Mar 23, 2017

View reviewed changes

strk suggested changes Mar 23, 2017

View reviewed changes

tboerger removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 23, 2017

sapk mentioned this pull request Apr 21, 2017

Check if missing/modified/unused deps in vendor and fix errors #1468

Merged

4 tasks

lunny reviewed Apr 21, 2017

View reviewed changes

lunny requested changes Apr 21, 2017

View reviewed changes

ethantkoenig force-pushed the test/integration branch from c12cd0d to e26283e Compare April 21, 2017 17:35

lunny approved these changes Apr 24, 2017

View reviewed changes

lunny removed the pr/wip This PR is not ready for review label Apr 24, 2017

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 Apr 24, 2017

appleboy reviewed Apr 24, 2017

View reviewed changes

ethantkoenig and others added 3 commits April 24, 2017 11:36

Integration test framework

0f746e7

udpate drone sign

66c8f3e

Formatting fixes and move router.go to routers/

12fe342

ethantkoenig force-pushed the test/integration branch from e26283e to 12fe342 Compare April 24, 2017 15:38

update sign for drone

5003a03

lunny merged commit c58708d into go-gitea:master Apr 25, 2017

sapk mentioned this pull request Apr 26, 2017

Rework SSH key management UI to add GPG #1293

Merged

5 tasks

ethantkoenig mentioned this pull request Apr 28, 2017

Acceptance/Regression Testing unfoldingWord/dcs#73

Closed

ethantkoenig deleted the test/integration branch April 29, 2017 14:13

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

		@@ -0,0 +1,2 @@
		#!/usr/bin/env bash
		"/home/ethantkoenig/.gopath/src/code.gitea.io/gitea/gitea" hook --config='integrations/app.ini' post-receive

Integration test framework #1290

Integration test framework #1290

Conversation

ethantkoenig commented Mar 16, 2017 • edited Loading

bkcsoft Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkcsoft Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

ethantkoenig Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

ethantkoenig Mar 17, 2017 • 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 commented Mar 16, 2017

ethantkoenig commented Mar 17, 2017

lunny commented Mar 17, 2017

strk commented Mar 17, 2017 via email

ethantkoenig commented Mar 17, 2017

ethantkoenig commented Mar 20, 2017 • edited Loading

strk commented Mar 20, 2017

ethantkoenig commented Mar 23, 2017 • 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

strk left a comment

Choose a reason for hiding this comment

ethantkoenig commented Mar 23, 2017 • edited Loading

andreynering commented Mar 23, 2017

andreynering commented Mar 23, 2017

strk commented Apr 20, 2017

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

bkcsoft commented Apr 21, 2017

ethantkoenig commented Apr 23, 2017

lunny commented Apr 24, 2017

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

ethantkoenig commented Apr 24, 2017

strk commented Apr 24, 2017

lunny commented Apr 24, 2017

ethantkoenig commented Apr 24, 2017

lunny commented Apr 25, 2017

ethantkoenig commented Mar 16, 2017 •

edited

Loading

bkcsoft Mar 16, 2017 •

edited

Loading

bkcsoft Mar 16, 2017 •

edited

Loading

ethantkoenig Mar 16, 2017 •

edited

Loading

ethantkoenig Mar 17, 2017 •

edited

Loading

ethantkoenig commented Mar 20, 2017 •

edited

Loading

ethantkoenig commented Mar 23, 2017 •

edited

Loading

ethantkoenig commented Mar 23, 2017 •

edited

Loading