Failing tests

[dimitri-xyz]

It seems updates to the Coinbase Pro API have broken many tests. I only see the first 10 of 21 tests passing right now.

[ericpashman]

A number of tests have been failing since before my last spurt of work in December. I don't recall how many precisely, but I just re-ran the test suite on master and none of the failures jump out at me as new.

We had some discussion about this previously, and I opened issue #7 in particular. All failing tests on master related to the WebSockets API (three of the ten failures) are fixed in my still unmerged channels branch. I had intended to move on to fixing the broken REST calls, but I got distracted before tackling it.

The channels branch has a somewhat improved test suite, as noted in the pull request, #10. (I get 7 of 45 tests failing right now; all of the failures are of REST functionality, untouched by me.) The new local tests of the WebSocket functionality that I added, e.g., tests parsing the JSON examples from the Coinbase Pro documentation, are what I had in mind to add for the REST calls. Writing tests like this is a good way to start figuring out why the current tests are failing.

[dimitri-xyz]

Thanks. Seems first step is pulling in your merge request and then working backwards to fix the other problems. I will work on that.

[ericpashman]

Let me know what you want to take on. I can probably find some time to jump back in and help whip everything into shape. And I think I might have some additional changes in my fork or a local repo.

[ericpashman]

Re-posting here after mistakenly creating a new issue. Here's the run-down on the test failures. These look like they're be straightforward to fix:

There are seven tests that consistently fail right now, all related to REST API calls. All of these tests also failed before the dependency set update.

One failure is an error parsing into the Account type, (via the getAccounts call; the failing test is labeled getUSDAccountLedger). The FromJSON instance is derived generically. It looks like Coinbase changed a JSON field from a number to a string?

Five of the other six test failures are HTTP 400 "Bad Request": "OrderValidation" errors, and the sixth (getOrderList) fails with the message "Received empty order list". Presumably these are all due to a common cause, which looks like it probably has to do with the order-creation action defined in the tests, creatNewLimitOrder [sic].

One or two other tests sometimes fail for me due to rate limiting. The docs show that the rate limit for public endpoints is three calls per second, so these tests are in fact likely to get rate-limited sometimes if we're not explicitly delaying them (are we?). Our tests must be strangely slow for this not to happen more often if we're not delaying the calls.

I'll get to work on this tomorrow, probably, unless you want to tackle it before then, @dimitri-xyz.

[ericpashman]

I looked at the six tests that all fail because the sandbox server doesn't like the orders we're submitting. The requests we're sending look fine to me, and nothing in the order-submission process has changed, so far as I can tell from the documentation.

Google shows a StackOverflow question citing the "OrderValidationError" message from a few weeks ago (with no answers), and an inaccessible IRC log in which somebody asked about it. No further leads.

I suspect this is something screwy going on with the sandbox server, which seems to happen from time to time. I emailed Coinbase Pro support about it; we'll see if I get a response.

I'll create a PR momentarily to fix the failing getUSDAccountLedger test; go there for details.

[ericpashman]

After following up with Coinbase support to provide a detailed example of an order-submission request that results in a successful order placement on the live server but an "OrderValidationError" on the sandbox server, I just got an email saying the issue has been marked as resolved and closed, with no further explanation.

The tests still fail on the sandbox server. So much for that.

[ericpashman]

Oh, and to be clear I did debug this pretty thoroughly and noticed nothing wrong with our HTTP requests encoding order submissions. In a couple of test examples, the live server accepted order submissions that were identical up to the necessary differences in URL and authentication headers. I don't think authentication is the issue, as other authenticated calls work on the sandbox server.

In short, I'm fairly confident the problem is on Coinbase's end, which makes this rather frustrating. A sandbox server that behaves differently from the live server and is simply broken for long periods is more trouble than it's worth.

One last thing: there two diverging code paths for making REST API calls against the live server and sandbox server, coinbaseRequest and realCoinbaseRequest in Coinbase.Exchange.Rest. I'm not sure whether this is a vestige of some past functionality, but it's redundant and ugly, as is much of the nearby code related to order submission (e.g., some request headers are set more than once), and could benefit from refactoring. It's also where the problem likely lies if indeed the failing tests are our fault rather than Coinbase's.