Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction tests are incorrectly passing; need updating to latest format #19033

Closed
danjm opened this issue Feb 11, 2019 · 0 comments
Closed
Assignees

Comments

@danjm
Copy link

danjm commented Feb 11, 2019

It seems that the tests found in go-ethereum/tests/testdata/TransactionTests/ and run via go-ethereum/tests/transaction_test.go are not actually running but appearing as if they are all running and passing.

The current structs in https://github.com/ethereum/go-ethereum/blob/master/tests/transaction_test_util.go were written to handle the form of the tests before ethereum/tests#374. With that PR, the structure of the json for each test changed. As a result, the following portion of the code returns nil for every test:

if err := rlp.DecodeBytes(tt.json.RLP, tx); err != nil {
		if tt.json.Transaction == nil {
			return nil
		}
		return fmt.Errorf("RLP decoding failed: %v", err)
	}

nil indicates a passing test.

I have put together a rough draft of how we can fix this https://github.com/ethereum/go-ethereum/compare/master...danjm:fix-TransactionTests?expand=1 Essentialy it requires updating the tests to work with the transaction test json as it is currently structured. There are a number of failing transaction tests on that branch, and I need to confirm whether those failures are real or the result of errors in the test code.

In the meantime, I wished to create an issue first because it seemed strange that this would have been a problem for so long. I want to confirm that there is not an intended reason for keeping the transaction tests in there current state? Other whether the preferred solution, at least in the short term, would be to return to using the tests of the old format?

@fjl fjl assigned fjl and holiman May 3, 2019
holiman added a commit to holiman/go-ethereum that referenced this issue May 5, 2019
karalabe pushed a commit that referenced this issue May 21, 2019
* tests: make transaction tests run again, fix #19033

* tests: refactor transaction tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@fjl @karalabe @holiman @danjm and others