add tr rlp to state tests

[winsvega]

this leaves the current state tests as it is but adds a tr RLP to the final test
so you could use both indexes and transaction rlp.
@fjl please check that you could parse this RLP and it is correct

use ./testeth -t StateTestsGeneral -- --filltests --checkstate to produce new format to the test repository

[pirapira]

I wait for @fjl and @winsvega to talk.

[fjl]

It's not sufficient to just add the transaction, cpp-ethereum also needs to use it for testing.

[winsvega]

there is whole bunch of code which rely on general state tests indexes.
such as
-t < trIndex> -v < valueIndex> -g < gasIndex>

that check a specific transaction during the test filling, debug, and execution.
expect section rely on those indexes. compare expect to post state function as well.
logic that deal with transaction section in state tests.

I don't think that now is a good time for a big refactoring like this. But we can include transaction rlp into final tests with one line. (which is basically an rlp representation of a transaction section)

[fjl]

I am not saying that expect indexes should be removed from the filler. It's OK to have the indexes there. But running the test without --filltests doesn't read the filler and doesn't know about expect section.

In ethereum/EIPs#176 (comment), I'm proposing to remove "transaction" from the test. It will remain in the filler, but "indexes" is useless without "transaction".

[fjl]

To check a specific transaction when not filling, you can always identify it by fork and index in the array.

[winsvega]

running the test without --filltests still need the indexes at the moment. you will have to remove this logic.
and replace -v -g -d options for running a specific test without --filltests. (while still keeping it for running a test when using filltests) also debug functions will have to be changed for a case without --filltests.
I am still don't think that it is a good idea to touch so many files before the release.

we should better focus on fuzz tests and test coverage for Metropolis.

[fjl]

you will have to remove this logic

I'm working on it ;)

I am still dont thing that it is a good idea to touch so many files before the release

This doesn't need to get in before Metropolis. It can wait.

[pirapira]

To me, the current situation is, many many things are broken (hive cannot run consensus tests because BlockChainTest/EIP150 directory is gone), and we have to get these fixed before Metropolis. It's not a good moment to change the final test format in a backward-incompatible way. I didn't realize -v -d -g options would be gone. A couple of weeks after Metropolis, we can change formats and start fixing all clients' test execution.

[winsvega]

the format was not changed. blockchain tests just moved to different directories.
ethereum/tests#197

we missed that other clients use static links to test folders

[pirapira]

@winsvega yes, things break when we just move directories. I don't think we can afford to break things now.

[winsvega]

could they just scan the folders for .json files?
basically now you don't have to specify network for a blockchain test depending on a folder name.
the test now has "network" field in .json

[pirapira]

They could but they did not. People don't do what we think they could. That's why things break for a change that, we think, should not matter.