Looking for feedback for upcoming httpexpect.v2

[gavv]

Problems with current API

The big problem with current API is that it's totally unclear from the source code what is the type of every object in a chained call.

Here is an example:

e := httpexpect.New(t, "http://example.com")

e.GET("/fruits/apple").                   // returns new httpexpect.Request
    WithHeader("If-Match", "something").  // returns method receiver (httpexpect.Request)
    WithQuery("key", "value").            // returns method receiver (httpexpect.Request)
    Expect().                             // returns new httpexpect.Response
    Status(http.StatusOK).                // returns method receiver (httpexpect.Response)
    ContentType("application/json").      // returns method receiver (httpexpect.Response)
    JSON().                               // returns new httpexpect.Value
    Object().                             // returns new httpexpect.Object
    ContainsKey("colors").                // returns method receiver (httpexpect.Object)
    Value("colors").                      // returns new httpexpect.Value
    Array().                              // returns new httpexpect.Array
    Element(0).                           // returns new httpexpect.Value
    String().                             // returns new httpexpect.String
    Equal("green")                        // returns method receiver (httpexpect.String)

Actually there are two kind of methods: ones that return the method receiver, and ones that return a new object for a nested value. However, it's hard to recognize what is the kind of a given method, and the user should consult the documentation or IDE for each one.

This makes the code unclear for those who are unfamiliar with the httpexpect API. People already mentioned this on reddit when httpexpect.v1 was announced there.

Suggested new API

I finally came up to an idea (actually not a new one) of the new API that will solve the problem. The new suggested API is based on lambdas. Less talk, more code. Here is the above example ported to new API:

e := hx.New(t, "http://example.com")

e.GET("/fruits/apple").
    WithHeader("If-Match", "something").
    WithQuery("key", "value").
    Expect(func (hr *hx.Response) {
        hr.Status(http.StatusOK)
        hr.ContentType("application/json")
        hr.JSON(func(ho *hx.Object) {
            ho.ContainsKey("colors")
            ho.Value("colors", func(ha *hx.Array) {
                ha.Element(0, func(hs *hx.String) {
                    hs.Equal("green")
                })
            })
        })
    })

The same may be written as:

e.GET("/fruits/apple").
    WithHeader("If-Match", "something").
    WithQuery("key", "value").
    Expect(func (hr *hx.Response) {
        hr.Status(http.StatusOK)
        hr.JSON(func(ho *hx.Object) {
            ho.Value("colors", func(ha *hx.Array) {
                ha.ValueEqual(0, "green")
            })
        })
    })

Or:

e.GET("/fruits/apple").
    WithHeader("If-Match", "something").
    WithQuery("key", "value").
    Expect(func (hr *hx.Response) {
        hr.Status(http.StatusOK)
        hr.JSON(func(ho *hx.Object) {
            ho.Path("$..colors[0]", func(hs *hx.String) {
                hs.Equal("green")
            })
        })
    })

The new rules are dead simple:

• every method returns its receiver
• assertions for nested objects (response body, map value, array element) accept function as an argument

Advantages:

• the objects types are obvious
• the object nesting is reflected in the source code
• the order of statements at the same nesting level doesn't matter

Disadvantages:

• the compatibility is broken
• increased usage of reflection in API (but not in user code)
• the code looks fancier

Since the new API forces to use package name much more often, it will be changed from httpexpect to hx (github.com/gavv/httpexpect/hx).

Migration process

• The httpexpect.v1 stable version (available on gopkg) will not be dropped and the compatibility will be preserved. I will not develop new features for it, but I will fix bugs and accepts pull requests.

• The master will be switched to a new API when it will be ready and usable. I didn't start working on the new API yet, so it will take some time.

• To allow incremental migration, every object in the new API will provide Compat() method returning an appropriate instance from the old API. Thus you can still use parts of code written for the old API.

• When the new API will be stabilized, the Compat() methods will be removed and the httpexpect.v2 stable branch will be created.

If you're currently using master (github.com/gavv/httpexpect), it's time to switch to a stable branch (gopkg.in/gavv/httpexpect.v1) to avoid breaks in near future.

Other (minor) breaking changes in v2

• Allow JSON vendor content type #22
• Unify and improve error reporting #35

Looking for feedback

Before starting working on the new API, I'm looking for any feedback and suggestions. You're welcome!

^{@acidvertigo @banux @EtienneBruines @gosilent @JamesMura @joohoi @jraede @kataras @lstep @mattes @mrLSD @nullstyle @oalmali @phifty @sofixa @typekpb @yudai}

[ghost]

Hello @gavv nice to see that you see the future of this library, but I will prefer that you shouldn't 'kill' the Builder pattern you have made so far, you can just put an interface with Response() or/and Request() or Object() to 'go back' to the previous builder, because the func(hr *hx.Response) doesn't seems to be a good idea for a testing library like this, you should target the ease-of-use

[mattes]

I absolutely love the current testing style. It's the main reason why we use this lib. Tests are fast and very readable.

[TheGrum]

If you do keep this new style for v2, consider creating type aliases for the functions. Then you can do:

e.GET("/fruits/apple").
    WithHeader("If-Match", "something").
    WithQuery("key", "value").
    Expect(hx.RespFn {
        hr.Status(http.StatusOK)
        hr.JSON(hx.ObjectFn {
            ho.Path("$..colors[0]", hx.StringFn {
                hs.Equal("green")
            })
        })
    })

Though you do have to know what the parameter name is, so making them all hp instead of hs/ho/hr might be better.

[gavv]

@kataras

I will prefer that you shouldn't 'kill' the Builder pattern you have made so far

Note that the new API still uses the builder pattern. The only change is that it uses nested assertions for nested objects.

you can just put an interface with Response() or/and Request() or Object() to 'go back' to the previous builder

I think it'll make the user code too magical.

@mattes

I absolutely love the current testing style. It's the main reason why we use this lib. Tests are fast and very readable.

I understand that brevity is an important feature of httpexpect. However, clarity is probably even more important, and it's missing currently.

Suggested API will preserve the current style as much as possible, with the only change of making nesting explicit. Of course, all existing shorthands that don't return nested objects can be used as usual, e.g. ContainsKey and ValueEqual:

e.GET("/fruits/apple").
    Expect(func (h *hx.Response) {
        h.Status(http.StatusOK)
        h.JSON(func(h *hx.Object) {
            h.ContainsKey("foo").ValueEqual("foo", 1)
            h.ContainsKey("bar").ValueEqual("bar", 2)
        })
    })

I'm still experimenting with various ideas, but so far the API suggested above seems to be the best compromise if you want short, but non-magical tests.

@TheGrum

If you do keep this new style for v2, consider creating type aliases for the functions.

Looks interesting, but I don't like that it hides the types again. I think users that need this can add such aliases by themselves.

Though you do have to know what the parameter name is, so making them all hp instead of hs/ho/hr might be better.

Anyway, even without such aliases it seems to be a good idea to use the same identifier for all nesting levels. I'll use it in new documentation and examples.

[ghost]

@gavv I totally feel you, it is impossible to change your mind right now, if you have an idea, just do it, I'll still support and use this library :)

[ybbus]

I just can support mattes and ghost's comment.

The old way looks much more readable.

Maybe you could add methods like AND() so you can go back from an expect chain and do another expect like:

Json().Contains("abc").Contains("def").And().Status(200)?

[gavv]

Well, no one supported the idea of the new API for quite a long time, so I think I should not break compatibility for solving problems that are not critical for real users.

If this will ever happen, it's likely should be a separate library.

Closing this for now :)