API test framework limitations

[theoreticalbts]

Apparently the test code in #1666 has been merged to develop. I have the following issues with this test code:

No documentation

How exactly do I run these tests? There needs to be a README.md or some documentation somewhere about how to execute the tests from a shell prompt. If I don't run them, then I will break them and not know, which leads to some problems:

• If no one runs them, eventually these tests will become so broken it will be impractical to post-mortem exactly what changes broke what test, meaning they are useless and will need to be rewritten from scratch.
• Since these tests use hard-coded output, it is very easy to make changes that break them.
• If one of my changes actually causes an end-user issue that could have been detected by these tests, if I don't run them then I won't know, and the tests won't serve their purpose of catching problems early.

No CI

Once the process for running these tests is understood by all developers, someone needs to add these tests to our CI so they run on every branch.

Hardcoded output is unsuitable

The fundamental issue with the design of these tests is that they have hard-coded output. Which means frequently they will be broken by minor changes. And usually the fix will be to simply copy-paste the actual output over the contents of the test. This adds unnecessary work to every commit that touches any part of the API.

Additionally, it introduces a more insidious problem than merely increasing developer workload and reducing agility. Adding this kind of testing causes developers to internalize through repetition that "these tests often break, the breakage means nothing and is quickly fixable by copy-pasting the new output into the test." Then, when real breakage does occur, it is quite likely that the affected developer(s), desensitized to test failures by frequent false alarms, will not closely investigate and instead merely copy-paste the new output into the test, which has always fixed this kind of issue before with no ill consequences -- until now! This time, a test which exposed a real problem is "fixed" by changing the test to accept the incorrect output!

This was the main problem with the test suite in BitShares 0.x. The boost::unit_test based testing of Steem (and BitShares 2.0 / Graphene) was implemented for this shortcoming.

Course of action

So there are three possible courses of action.

• (a) Accept some/all of the above problems and let the code stand.
• (b) Remove these tests.
• (c) Change the tests so that they don't suffer from these problems.

I don't like option (a), since based on experience, the fragility of these tests means they will prove over time to be a productivity sink and provide a false sense of security. Option (c) will effectively mean a complete re-architecture of the design and rewrite of the tests, so effectively Option (b) is the first step on the road to Option (c).

Therefore, I recommend removing these tests and redesigning the entire API testing approach to address these issues.

[mvandeberg]

I was trying to start this discussion in #2042

The PR was merged because I was told it was blocking further development and because it was not running in CI, nor impacting steemd in any way, there was no harm in merging it while we had these discussions.