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

Question: Fixing the package interface #13

Open
dnephin opened this issue Dec 21, 2017 · 2 comments
Open

Question: Fixing the package interface #13

dnephin opened this issue Dec 21, 2017 · 2 comments

Comments

@dnephin
Copy link

dnephin commented Dec 21, 2017

Hello, I'm wondering if you are planning on fixing some of the problems with the package interface.

I think some of the main problems are:

  • way too many assertions
    • everything is duplicated as Foo and Foof, and essentially duplicated again in require (since the code is generated this is more of an issue for users and less for maintenance, also less code generation wouldn't hurt).
    • HTTP and JSON assertions belong in a separate package for testing http requests
    • lots of confusing overlap (Equal, vs Exactly, Zero/Nil/Empty/Len(x, 0), InDelta/InEpsilon)
    • unnecessary assertions (Fail, FailNow, True, and False provide nothing over the standard testing.T, NotPanics is effectively a no-op)
  • inconsistent parameter placement (Contains/Len/EqualError all accept the expected value as the second arg, but most other assertions accept the expected as the first)
  • require behaves the way most people would expect assert to behave. assert is more of a "check"
  • Equal uses reflect.DeepEqual() which has many shortcomings, which can be fixed by using https://godoc.org/github.com/google/go-cmp/cmp
    • the multiline diffs from difflib show every pointer as being different because of the address
    • there's no way to ignore some fields on structs that may always be different (ex: timestamps)
    • comparisons can fail because of unexported internal fields

I have been working on a library which I believe fixes all of these issues: gotestyourself/gotest.tools#34

In addition to fixing these issues, some of the benefits to this design are:

  • now that t.Helper() exists there's no need to calculate line numbers, so all that code is removed
  • extensible assertions. users can write custom comparisons (func() (bool, string)) that return success/failure message. This also removes the need for code generation, and still supports both Fail and FailNow
  • more type safe asserts (Equal uses actual equality (==), Compare uses go-cmp for comparing complex types)
  • Assert() supports booleans as well as Comparison, so trivial compares of booleans, or x != nil can be done inline, and the source is printed as the failure message(ex: "assertion failed: x != nil is false")

I'm writing a tool that translates testify.Assert into assert.Assert() using go/ast rewriting, to make it trivial to migrate a project to the new interface.

Since you are spending time on this fork, and looking for maintainers I'm wondering if our goals are aligned and we might be able to share some work.

@dignifiedquire
Copy link

Thank you for reaching out, I didn't know about the tooling from gotestyourself and they look really nice.

I haven't had time to go through each of the problems, but some general thoughts

  • yes, lets try and combine forces
  • t.Helper is only available in later go versions, currently this package supports down to 1.4 so this is a longer discussion probably
  • it would be great to reduce the api surface and amount of code gen
  • I wonder if assert and require could be combined (I personally always use require to avoid silent failures anyway)

@philtay
Copy link

philtay commented Dec 21, 2017

+1 to have @dnephin on board. we should somehow merge the projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants