Hacker News new | ask | show | jobs
by randomdata 804 days ago
The example was okay, but incomplete. It seems we have enough information now, so:

    type quotaNetworkFailure struct{ quotaSuccess }
    func (quotaNetworkFailure) UpdateQuota() (bool, error) { return false, quota.ErrNetworkFailure }

    func TestTransaction_QuotaUpdateNetworkFailure(t *testing.T) {
        tx := &Transaction{
            Auth:         authSuccess{},
            Payment:      paymentSuccess{},
            Storage:      storageSuccess{},
            Quota:        quotaNetworkFailure{},
            Notification: notificationSuccess{},
        }
        _, err := tx.Request("12345", "valid-token", "product123", 100.0, "USD", []byte("file data"))
        if !errors.Is(quota.ErrNetworkFailure, err) {
            t.Errorf("unexpected error: %v", err)
        }
    }
While still contrived, surely you agree that is much more understandable, not to mention a whole lot easier to write? Now we know why something might fail, and the reader learns what they should look for when handing a network failure.

Yes, okay, you got me. That isn't the HTTP service anymore. But it should have never been. The transaction processing is not an HTTP concern. In fixing that, now you can make the HTTP end of things far less obtuse:

    var errGeneralError = errors.New("general error")
    type transactionFailure struct{}
    func (transactionFailure) Request(id, token, productID string, amount float64 /* yikes */, currency string, file []byte) {
        return false, errGeneralError
    }

    func TestBuy_Failure(t *testing.T) {
        srv := httptest.NewServer(&Server{
            Transaction: transactionFailure{},
        })    
        defer srv.Close()

        c := client.New(srv.URL)
        _, err := c.Buy("product123")
        if !errors.Is(transaction.ErrFailed, err) {
            t.Errorf("unexpected error: %v", err)
        }
    }
But, this is still not a good example of input. Input doesn't matter when the network fails. Let's say we want to cover the case where the product ID being purchased doesn't exist instead:

    type transactionNoProduct struct{}
    func (transactionNoProduct) Request(id, token, productID string, amount float64 /* yikes */, currency string, file []byte) (bool, error) {
        if productID != "product123" {
            return true, nil
        }
        return false, transaction.ErrProductNotFound
    }

    func TestBuy_ProductNotFound(t *testing.T) {
        srv := httptest.NewServer(&Server{
            Transaction: transactionNoProduct{},
        })    
        defer srv.Close()

        c := client.New(srv.URL)
        _, err := c.Buy("product123")
        if !errors.Is(transaction.ErrProductNotFound, err) {
            t.Errorf("unexpected error: %v", err)
        }
    }
There is still plenty of room for improvement here that I will cut short as who cares for an HN comment, but man, I don't see how anyone can think that mockery monstrosity is preferable to... anything else. Where is it actually useful?
1 comments

Its essentially the same thing with just different sytnax.

  func (transactionNoProduct) Request(id, token, productID string, amount float64 /* yikes */, currency string, file []byte) (bool, error) {
          if productID != "product123" {
              return true, nil
          }
          return false, transaction.ErrProductNotFound
      }
vs

    mockValidator.On("Request", "12345", "valid-token", "product123", 100.0, "USD", mock.AnythingOfType("[]uint8")).Return(false, services.ErrProductNotFound)
The nice thing about these mocker libraries is they integrate with the testing framework to show nicely formatted error messages about what exactly went wrong. Something you would have to build yourself otherwise.

Essentially if you take your concept further and try build a general purpose library for building out those stub functions you'll get something that looks like the mock library. They just give you all the utilities out of the box and you can apply to the granularity you see fit.

> Its essentially the same thing with just different sytnax.

Indeed. Hence why I said that they can both be considered stubs (or whatever you want to call them). We already went over this. But one comes without all the devastating baggage of mockery, which is a pretty big deal.

> The nice thing about these mocker libraries is they integrate with the testing framework to show nicely formatted error messages about what exactly went wrong

Except you also need all of that same information when the same thing happens in production – for your logs, the end user, or even plain functionality – so you haven't gained anything. You're just duplicating efforts for no reason.

That or, knowing the average programmer, they'll just haphazardly throw in that information and not test it since they think mockery gives them the information anyway, and then the future programmer won't be able to figure out when it does some particular thing, which would have been avoided if properly documented in test. Indeed, programmers by and large seem to hate other programmers for some reason. That doesn't seem like a good justification, though.

> Essentially if you take your concept further and try build a general purpose library for building out those stub functions you'll get something that looks like the mock library.

It may be that mockery is just a poor implementation of the concept. There are some obvious improvements that could be made to mockery to at least begin to help with its grievous problems, so we know it is not as good as it could be. Is there a tool that does it better?

Or is this a classic case of someone taking DRY much too literally? As the Go proverb goes: A little copying is better than a little dependency. Is an abstraction truly warranted in the first place?