Regression Tests are not valid until we check for specific exceptions

[bytemaster]

Right now a test checking for an exception will pass regardless of what type of error actually occurred. For the tests to be valid every exception that we test for needs to be explicit.

[vikramrajkumar]

Related: #117

[theoreticalbts]

Here is my proposal for an exception hierarchy:

• graphene::chain::chain_exception will be the root of anything that can happen in the chain DB.
• Exception opcodes will be six digit numbers of the form 3xxyyzz.
• The initial digit 3 is inherited from the BTS 0.x exception numbering scheme.
• The next two digits xx represent a phase of validation.
• The meaning of the next four digits yyzz may depend on xx.
• For ops which represent operations, xx has the following meanings for yy and zz:
• yy is the operation number (i.e. which member of the static_variant in operations.hpp)
• zz is the number of the exception within the operation

Compile-time template automagic will be used to enforce the correct value yy (i.e. instead of hard-coding numbers, it will be done based on the compile-time-known static variant tag for the requrested type). Thus, reordering the operations will change exception codes!

Currently the following values of xx are defined (in my private branch for this refactor):

• xx = 01: Database query exception. Thrown by methods which are raw queries to the chain database, e.g. get_recent_transaction().
• xx = 02: Block validation exception. Thrown during evaluation of block-wide concerns (e.g. incorrect signing witness ID).
• xx = 03: Transaction validation exception. Thrown during evaluation of tx-wide concerns (e.g. tx is expired).
• xx = 04: Operation validation exception. Thrown by operation during validate().
• xx = 05: Operation evaluation exception. Thrown by some evaluator in evaluate().

[theoreticalbts]

Note, many operations throw FC_ASSERT( fee_amount > 0 ). My above-proposed scheme would assign different exception types / codes to each operation's version of this check.

This is actually a feature, not a flaw.

If the primary purpose of exceptions is to allow the testing framework to confirm that a specific assert exception triggers under the conditions set up by the test, then it makes sense to have each exception type thrown by a very small number of places in the code -- ideally in only one place. This makes each test able to confirm with high specificity that the observed exception does in fact indicate the desired assertion has failed, and not some other assertion.

[theoreticalbts]

If it is desired to have a general insufficient_fee_exception which is used for all fee_amount > 0 checks, it is possible to do so by making each op's exception for this condition descend from a base class insufficient_fee_exception. This allows unit tests which test for specific ops to catch the fee_amount > 0 exception for that op, and then other code which wants to catch generic insufficient fee condition can catch the base class.

It seems hard to figure out an actual use case which would actually want to catch the base class, however -- which suggests this feature need not be implemented.

[theoreticalbts]

Many commits progress, e.g. d4e4854 9c4ac2e 54103da 36e155b 35ab119 1b5a7cb

[vikramrajkumar]

This issue was moved to bitshares/bitshares-core#32