Problems with tests

[ericpashman]

There are numerous problems with the current test suite. Test coverage is poor, and the existing tests don't actually test much of what you might think they do (see my comment in #6 for an example).

The most immediate problem is that the existing tests are dangerous (and not obviously so), because if the COINBASE_PRO_SANDBOX environment variable is set to FALSE then the test suite gets run on the live server, using whatever real money somebody might have in their trading account. Submitting real-money orders obviously should not happen when running the test suite. We should ensure that all tests only run on the sandbox server until we can sit down and ensure nobody can lose money if they unwittingly run the tests with an API key for a funded trading account.

It's also a bit obnoxious that one must create a Coinbase Pro account, generate an API key, and set some environment variables just to run the test suite. If it's possible (I don't recall how Coinbase's sandbox site works), we should create a dummy account for the project on the sandbox site and hard-code some credentials into the test suite so that anybody can just show up, download the code, build it, and test it.

I'll plan to do a bit of work on this in the coming days. Meanwhile, let's start thinking how else we can improve the tests.

[dimitri-xyz]

Thanks for creating this issue! The tests definitely need to be improved.

I also agree that the tests are dangerous. I think the user needs to explicitly let us know if she wants to run tests on the live server, so that it doesn't happen by accident. At the same time, I have seen discrepancies between the sandbox and the live live environments (the lack of data is the most annoying), so I think it is wise to retain the ability to run tests on the live server. (Maybe this means the tests should be parametrized on the funds they use and market conditions? E.g. current price)

Having to create a Coinbase Pro account to run the tests is definitely obnoxious, maybe that should also be a test option. We can give the test user the option to use a dummy account? We should be a little careful here. I am uncomfortable making changes that makes the tests less reliable. Using a shared account would have that effect as multiple users may be using the same account simultaneously. So, I think we should retain the ability to run the test on a dedicated test account. In fact, I think that is the standard staging environment: a testing account without much money on the live server.

[ericpashman]

I don't think a public test account on the live server is workable because (1) it will have to be a real account that passes Coinbase's AML/KYC checks, and publishing the credentials would surely violate the TOS; and (2) an unfunded account can't be used to test any functionality that requires funds (e.g., order submission).

We should probably write some tests that are designed for users to run against the live server with small amounts of real money via their own accounts, but I think it's more important first simply to make sure that we're thoroughly testing everything we can on the sandbox server. The obvious way to do that is to write tests which themselves generate all the activity we want to see, making the server produce the messages we want to test (e.g., submit overlapping orders to make a trade occur in order to test match messages). Note that we may need two tests accounts to make this work well if the sandbox server's self-trade prevention is enabled.

Another option is to mock and test a stream of messages by recording real activity on the live server. That shouldn't be too difficult and may reveal some things that testing on the sandbox server doesn't. It also would give us the ability to do regression testing by keeping around messages or sequences of messages that have led to errors in the past.

See my further comments about the existing tests at #6.

[dimitri-xyz]

Sorry, I wasn't clear! The staging environment should not be provided by us (no "public test account on the live server"), but I assume many people running the library will have one. Some of the current tests can be very useful to people who have such "staging" accounts.

You are right that we should as much as possible run test on the sandbox. It is clear the tests need some TLC. I may have time to look at this during the weekend.

[ericpashman]

To pick up from the discussion in #6, yes, I've had in mind tests aimed at checking the correctness of the library's functionality during development and installation.

I agree that there are other valuable use cases for tests, particularly on the live server. We'll just have to be clear about which tests are intended for which purposes, or at least which are safe to run in the usual context of a write-test-debug development pattern, or of a user downloading, installing, and testing the code, and which are not. Perhaps we should just start with a Test.Safe module and a Test.Unsafe module, with only the former called by Test.Main.hs by default so that cabal test, stack test, etc., can be run without any gotchas.