Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test utility lib #3498

Open
Kubuxu opened this issue Dec 12, 2016 · 12 comments
Open

Test utility lib #3498

Kubuxu opened this issue Dec 12, 2016 · 12 comments
Labels
need/analysis Needs further analysis before proceeding need/community-input Needs input from the wider community

Comments

@Kubuxu
Copy link
Member

Kubuxu commented Dec 12, 2016

As follow up to: multiformats/go-multibase#6

It might be worth to think a bit about introduction of test utility lib so we don't have to use 3 lines for to pass through an error or to compare two values. Space on a screen is a resource too that shouldn't be wasted.

I would also like for use to settle on one lib so it is uniform across codebases.

https://github.com/stretchr/testify looks quite nice.

What is your opinion?
Should we gx'ify the lib if we start using some?

@Kubuxu Kubuxu added the need/analysis Needs further analysis before proceeding label Dec 12, 2016
@Kubuxu Kubuxu added the need/community-input Needs input from the wider community label Dec 12, 2016
@whyrusleeping
Copy link
Member

I'm pretty against external test libs in general, if everyone else disagrees with me, i could be convinced otherwise, but in general i like writing vanilla go tests.

@wigy-opensource-developer

When it comes to me, I like redhead girls. But besides personal taste, why do you dislike external test libs? 😋

Back to serious, you mentioned vanilla tests. Does that mean you do not like to extract methods reusable all around test code? Or you just do not want to depend on a library maintained and vendored independently from Protocol Labs?

@hsanjuan
Copy link
Contributor

I agree with @whyrusleeping. I love rspec, and I also landed on testify looking for something similar. But Go is Go. We cannot have magic mocks like in Ruby. So it wasn't going to help so much and I went for less dependencies and vanilla tests, which do the job all the same, albeit with some extra lines. Almost everything in Go takes extra space on screen, so I find it a weak reason to introduce idioms and [test-only]-deps.

@wigy-opensource-developer
Copy link

wigy-opensource-developer commented Dec 13, 2016

You are right, @hsanjuan. Assertions for equality, their semantics, their options are so common case that it should be in the Go standard library itself. That would save developers learning new idioms for these common tasks in each project.

But Go is Go, so what options do we have to reduce noise in the test code?

Grown up on http://xunitpatterns.com/ I really treat test code as first-class citizen that needs the same level of refactoring and maintenance as production code to avoid slow-down in the long-run as new features are added and bugs are fixed.

@wigy-opensource-developer

@Kubuxu Looking at multiformats/go-multibase#10 I got the impression that introducing abstractions for test code seems to be a tough call in this team? 😉

@hoenirvili
Copy link
Contributor

This project needs to stop using the testing pkg from std and move to a more mature and serious one like https://labix.org/gocheck.
I'm also a serious contributor on another big open source project Juju, which is much bigger than the go-ipfs so it uses this library I mentioned and also this custome one https://github.com/juju/testing.

Take a look on how cleaner and maintainable their tests are compared to this project.

I'm really not a fain of writing code like this

 n, err := pkg.Reader(from)
 if n != somNum || err != nil {
   // test fail  
 }
 if err == errSepcialCase {
  // fail also 
 }

 if n == someExpectedNum {
   // test passes
 } 

comparing to this one

  n, err := pkg.Reader(from)
  c.Assert(err, gc.IsNil)
  c.Assert(n, gc.DeepEquals, someExpectedValue) // this checks also the type and the value

And take into account, this is a trivial example, what do you do when, you must check for more complex logic code?

Writing vanilla go tests are good for very small projects but when you are going bigger and bigger and adding complexity you need to start reconsider and move to a more robust testing framework implementation.

If my PR which I referenced in there, will be merged I would happily and gradually move all the test code that has been yet written.

@whyrusleeping
Copy link
Member

@Kubuxu @hsanjuan @lgierth want to weigh in here?

Like i said, I could be convinced to change my position, but it needs to be a better argument than "Look, we can save a few lines of code by using this fancy function"

@hoenirvili I'd be interested for you to give an example or two of what you consider "more complex logic code"

That said, Given interactions i've had with both testing libs being discussed here, i do prefer testify. In general it seems to get in the way less than gocheck, and seems to be more well maintained.

@Kubuxu
Copy link
Member Author

Kubuxu commented Mar 21, 2017

My main requirement for testing lib is for it to have value added. If something stops us from writing descriptive test failure messages it is value reduced.

@hsanjuan
Copy link
Contributor

If something stops us from writing descriptive test failure messages it is value reduced.

Is that an argument against testify/gocheck @Kubuxu ?

If having a helper testlib is going to prevent re-opening this discussion from time to time when a contributor wants to add extra tests using one, then let's pick one (but only one). Hopefully this translates into making contributor's lives easier and becomes positive for the project.

A weak argument in favour of gocheck is that it does not seem to have sub-dependencies (so it's easier to Gx). I don't mind which one we pick. @wigy-opensource-developer which one is your favorite?

@Kubuxu
Copy link
Member Author

Kubuxu commented Mar 23, 2017

It was argument from other PR CR where switch to testing lib caused a descriptive error (setting something something failed) to be replaced with assertion failed: expected "true", got "false"

@wigy-opensource-developer

Thank you for asking. The only project I use golang for is the IPFS, so I do not have a strong opinion in favor of any test libraries. I just had the general feeling that when refactoring test code to reduce duplication, this project would extract the same kind of helper methods like any other projects would do in golang.

@whyrusleeping
Copy link
Member

Alright, I think I will concede to using testify with the following stipulations:

  • Refactored tests have strictly more descriptive output, and the readability is improved. I will push back on rewriting tests if i feel that switching to a new test framework is not actually improving things.
  • If we make an extra package of 'test helper utilities' (like @hoenirvili pointed out in juju). We should make sure that the dependencies are minimal. Having it depend on other libs makes updating through the tree of packages much more complicated

@hsanjuan hsanjuan removed their assignment Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/analysis Needs further analysis before proceeding need/community-input Needs input from the wider community
Projects
None yet
Development

No branches or pull requests

6 participants