Gitea: Testability Strategy

[bkcsoft]

Reference #2293

Basically, we need more tests :smile:

Should this go in go-gitea/proposals instead?

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[xinity]

:+1: for go-gitea/proposals

[tboerger]

And what do you want to propose? :D

[andreynering]

I think this is a big topic, so we should break it in small parts.

Unit tests

First of all we should have more unit tests. These tests should be memory only (e.g.: they shouldn't touch the database, git, and if possible even the filesystem). They are tests for isolated parts of the codebase.

Integration testing

This is harder, here we have to simulate HTTP requests and check if the behavior is correct. I think there are two main difficulties:

How to deal with a database

• Mocking the database
• We can test against a real database

I prefer to use a real database, since mocking will not catch problems like wrong SQL sintax, constraints violations, etc. #86 is a example that uses this approach. We can use a memory database for performance.

How to deal with Git

• We can mock the interface that talk to git
• We can write scripts to generate repositories and use real Git

In this case I prefer mocking since it will be easier than the alternative.

---

After we have a consensus it should be write in a formal document.

[strk]

+1 for real database, and ideally against all the supported and available databases systems
And also I think using real git would be better

Should this go under go-gitea/proposals, btw ?

[metalmatze]

Just want to add that I like to be mostly stdlib for unit tests.
Nevertheless I find https://godoc.org/github.com/stretchr/testify/assert extremely useful to prevent a lot of boilerplate.

val, err := somefunc()
if err != nil {
    t.Error(err)
}
if val != "foobar" {
    t.Error("val is not foobar")
}

would become:

val, err := somefunc()
assert.Nil(t, err)
assert.Equals(t, "foobar", val)

That's it.

[andreynering]

@metalmatze I agree. Go Convey was the convention on Gogs, but I also prefer testify/assert. That's another thing we should decide.

[makhov]

Convey is awful.
@metalmatze, note that testify/assert has methods assert.Error and assert.NoError to test errors

[metalmatze]

@makhov Even better! :smile: :+1:

[lunny]

Unit Tests first, that's great idea. And let's send more unit tests PRs.
About the unit test framework, we have to determine one ASAP.

@makhov, do you have time on code review PRs? If you like, I want to invite you to maintainers team.

[makhov]

@lunny right after somebody explain me what happens :)

[lunny]

@makhov Welcome back! I have moved you from Advisor Team to Maintainers Team.

[odinuge]

Isn't pure golang testing an equally good approach to unit testing? The assertion approach works, but in my opinion, the normal go-one works better. Eg. it forces the user testing to handle errors, instead of just panicking.

Ofc. they both comes with some different pros and cons, but the most important thing (as @lynny states) is to agree on "one" way to do it, and to _actually_ write the tests! :)

[makhov]

We use testing with testify/assert and it works for us.

[strk]

testify/assert works for me (never used), but have no objection if anyone wants to provide bare testing tests too

[lunny]

So let's use github.com/stretchr/testify, and anything else needed for unit tests?

[tboerger]

What about goblin? I really like this lib, especially with my BDD background on ruby from the past ;)

[metalmatze]

I think something like Goblin could be pretty good for integration tests. But for simple Unit Tests I'd prefer something very basic like stdlib with testify.

[bkcsoft]

@andreynering

First of all we should have more unit tests. These tests should be memory only (e.g.: they shouldn't touch the database, git, and if possible even the filesystem).

I disagree, unit-tests are optimal for testing db/git/fs, since they test a specific thing and not End-To-End (Integration tests) they would run fairly fast, and _only once per code-path_ (which is almost impossible with e2e-tests) :slightly_smiling_face:

[andreynering]

@bkcsoft OK, I agree, I think it's OK to touch Git or the DB with you are actually testing things that touch them.

[lunny]

I remember one PR has moved goconvey already?

[bkcsoft]

@lunny As previously mentioned, unit-tests will still use testify/assert while integration-tests could use goconvey (even though I don't like that lib...)

[andreynering]

I vote to use only testify/assert.

[lunny]

^

[sapk]

We could close this one ?

[lunny]

I think yes.