Remove Convey from tests

[litleleprikon]

In my opinion in Moira project convey tests are used in wrong and inconvenient way. I propose to discuss the perspective to remove Convey tests and rewrite tests using default testing.T.

Previous discussion:

You decided not to use Goconvey, even though all other tests in this file are Goconvey tests. Why?

Originally posted by @beevee in https://github.com/moira-alert/moira/pull/452

@beevee I believe that we decided to decrease usage of convey and finally remove convey at all, aren't we? I used the standard testing library to not increase the usage of convey. If you think that we don't need to refuse convey I can change this tests to use convey.

Originally posted by @litleleprikon in https://github.com/moira-alert/moira/pull/452

I remember something like this:

1. @titusjaka casually said that Goconvey sucks, and we should get rid of it. Everybody took it as an order, without much thought.
2. Later we discovered that magic feature of independently-run nested Convey clauses, and decided that it was interesting, and probably some of our tests rely on this behavior.

We didn't have any serious discussion after that. At least, I don't remember coming to a decision.

Here's what I think:

1. Goconvey Web UI as a useless fad, and I can easily live without it.
2. Nested Convey clauses that run independently is a powerful feature, even if we don't use it to the full power in most of cases right now.
3. A good-quality assertion library is a must. It makes test code so much more readable.

I don't think we have a clear plan right now. We don't have a replacement assertion library. Replacing our old tests this way, PR by PR, will take indefinite amount of time. As I see it, in a year, we'll just have 95% tests written in Goconvey, and 5% plain Go tests. Which is not a disaster, of course. Just a mess.

Originally posted by @beevee in https://github.com/moira-alert/moira/pull/452

I cannot disagree with your arguments but I have issues that I discovered during development:

• Some of our tests define variables outside of first convey call and this variables became the shared state between "independent runs" of convey test cases. That behaviour is predictable but can lead to misunderstand of test flow. This behaviour is also can be hard for new contributors that have experience with default testing in Go. You can check the example here https://play.golang.org/p/1XtHF_QT8IW

• Convey in my opinion is not so convenient to use with table-driven testing and provides increased testing time because for every test case convey rerun all code before the test.

• Asserts of convey cannot be used outside of convey context that force us to use convey even if we don't need independent runs of test cases. https://play.golang.org/p/QEz3HfBbv2A

As for now I do not see the easy way to remove convey from Moira. If we will decide to refuse the convey we can try to use convey less in our tests during development and remove convey runs when we edit existing tests but in any way the transition will take a lot of time.

Originally posted by @litleleprikon in https://github.com/moira-alert/moira/pull/452

Goconvey is not the only assertion library on the block. We can find another one. Testify has assertions: https://godoc.org/github.com/stretchr/testify/assert

Originally posted by @beevee in https://github.com/moira-alert/moira/pull/452

Yep I know about assertions in testify. My comment was about the inconvenient usage of convey asserts. I mean that even if I want to write the test without convey I have to use convey to be able to write assertions using So.

Originally posted by @litleleprikon in https://github.com/moira-alert/moira/pull/452

[titusjaka]

I checked 3 random projects and they all use testify 😄

• https://github.com/containous/traefik/blob/9ef4f47ba01fdd45547957b7be6145bf7bab68f2/go.mod#L79
• https://github.com/hashicorp/consul/blob/42dbf7f62342cd198f10147d644c707ae5657fb3/go.mod#L73
• https://github.com/helm/helm/blob/456eb7f4118a635427bd43daa3d7aabf29304f13/go.mod#L49

[litleleprikon]

@titusjaka I do not think that "everyone is using testify" is a valuable reason to remove Convey. As for now we have a situation in which our tests are not using Convey in correct way. On the other hand removing the Convey is a long and hard task. We want to define the easiest way to make our tests better and quicker. Can you provide another disadvantages of Convey that allow us to decide to remove Convey?

[litleleprikon]

IMHO Convey also is not intended to use in benchmarks and table-driven tests. We have a table-driven tests that use convey and this way of testing is much slower.

[beevee]

@titusjaka assume that today somebody released a supercool new testing library that is 100× better than testify. Imagine that you came to Helm or Consul repo and created an issue telling them to drop testify and migrate all their tests to supercool. What kind of response would you get?

Talk is cheap. Migrating all our tests to another testing library is a considerable effort. "All the cool guys use it" is not a valid argument.

[titusjaka]

Frankly speaking, I did not mean to switch from convey to testify just to change one lib for another. I suggested to use it as a good gateway for Open-Source newbies, who wants to start contributing but don't have enough time for huge issues.