cleanup json crate by niklasad1 · Pull Request #11027 · openethereum/parity-ethereum

niklasad1 · 2019-09-06T13:31:52Z

I got annoyed that some of the types had documentation that said only for tests which weren't the case. So I did a cleanup, sorry for a big PR

It does the following:

converts json to rust 2018
adds feature test-helpers
moves all tests helpers to test-helpers mod
removes duplicated types
some style fixes

dvdplm · 2019-09-08T11:49:08Z

+	pub static ref SKIP_TEST_STATE: SkipStates = {
 		let skip_data = include_bytes!("../../res/ethereum/tests-issues/currents.json");
-		ethjson::test::SkipStates::load(&skip_data[..]).expect("No invalid json allowed")
+		SkipStates::load(&skip_data[..]).expect("No invalid json allowed")


debris

So initially ethjson crate purpose was to load only json test files. Later on this functionality was extended. That said, I'm happy with all the changes and simplifications that you've made :)

write something here....

Co-Authored-By: David <dvdplm@gmail.com>

dvdplm · 2019-09-10T20:06:18Z

 pub struct TransactionTest {
 	pub rlp: Bytes,
-	pub _info: ::serde::de::IgnoredAny,
+	pub _info: serde::de::IgnoredAny,


Need docs or annotation.

dvdplm · 2019-09-10T20:06:36Z

-	pub _info: ::serde::de::IgnoredAny,
+	pub _info: serde::de::IgnoredAny,
 	#[serde(flatten)]
 	pub post_state: BTreeMap<ForkSpec, PostState>,


missing docs

…he-right-place * master: cleanup json crate (#11027) [spec] add istanbul test spec (#11033) [json-spec] make blake2 pricing spec more readable (#11034) Add blake2_f precompile (#11017) Add new line after writing block to hex file. (#10984) fix: remove unused error-chain (#11028) fix: remove needless use of itertools (#11029) Convert `std::test` benchmarks to use Criterion (#10999) Fix block detail updating (#11015) [trace] introduce trace failed to Ext (#11019) cli: update usage and version headers (#10924) [private-tx] remove unused rand (#11024)

* master: (70 commits) ethcore: remove `test-helper feat` from build (#11047) Include test-helpers from ethjson (#11045) [ethcore]: cleanup dependencies (#11043) add more tx tests (#11038) Fix parallel transactions race-condition (#10995) [ethcore]: make it compile without `test-helpers` feature (#11036) Benchmarks for block verification (#11035) Move snapshot related traits to their proper place (#11012) cleanup json crate (#11027) [spec] add istanbul test spec (#11033) [json-spec] make blake2 pricing spec more readable (#11034) Add blake2_f precompile (#11017) Add new line after writing block to hex file. (#10984) fix: remove unused error-chain (#11028) fix: remove needless use of itertools (#11029) Convert `std::test` benchmarks to use Criterion (#10999) Fix block detail updating (#11015) [trace] introduce trace failed to Ext (#11019) cli: update usage and version headers (#10924) [private-tx] remove unused rand (#11024) ...

* [json]: cleanup write something here.... * nit: commit new/moved files * nit: remove needless features * nits * fix(grumbles): use explicit import `DifficultyTest` * fix(grumbles): remove needless type hints * fix(grumble): docs `from -> used by` Co-Authored-By: David <dvdplm@gmail.com> * fix(grumbles): use explicit `imports` * fix(grumble): merge `tx` and `tx_with_signing_info` * fix(grumbles): resolve introduced `TODO's`

* ethcore/res: activate Istanbul on Ropsten, Görli, Rinkeby, Kovan (#11068) * ethcore/res: activate Istanbul on Ropsten block 6485846 * ethcore/res: activate Istanbul on Goerli block 1561651 * ethcore/res: use hex values for Istanbul specs * ethcore/res: fix trailing comma * ethcore/res: be pedantic about EIP-1283 in Petersburg and Istanbul test specs * ethcore/res: activate Istanbul on Rinkeby block 5435345 * ethcore/res: activate Istanbul on Kovan block 14111141 * ethcore/res: fix kovan istanbul number to 0xd751a5 * cleanup json crate (#11027) * [json]: cleanup write something here.... * nit: commit new/moved files * nit: remove needless features * nits * fix(grumbles): use explicit import `DifficultyTest` * fix(grumbles): remove needless type hints * fix(grumble): docs `from -> used by` Co-Authored-By: David <dvdplm@gmail.com> * fix(grumbles): use explicit `imports` * fix(grumble): merge `tx` and `tx_with_signing_info` * fix(grumbles): resolve introduced `TODO's` * [json-spec] make blake2 pricing spec more readable (#11034) * [json-spec] make blake2 pricing spec more readable * [ethcore] fix compilation * Update JSON tests to d4f86ecf4aa7c (#11054) * new ethereum consensus tests, #10908 * Update JSON tests to 725dbc73a This PR reverts the controversial changes of the previous PR and skips the failing tests. Maybe I misunderstand the suggested workaround of putting the fix under `#[cfg(test)]` but it seems odd to run different code in production than we run in tests. Instead here I suggest we skip the failing tests with the argument that we do not wish to fix this issue (at least not at this time) because it does not affect us. If I am wrong, and I likely am, I look forward to hearing why and what a better approach to updating the state tests is. Branched off #10923 ref #10908 * Update json test commit to 1dc9d20e97165708f7db0bbf2d1a87a6b4285827 * Fail with error message * Handle missing r, s, v params in json tests Light cleanup of json test runner * Include the path to the test file * Handle new `postState` format: string or map Sort out tests Missing docs * WIP * Include test-helpers from ethjson * Sort out new paths * Remove dead code * Fix warnings stemming from code called only from macros Skip failing tests in stRevert/ and stTransactionTest/ (too course a filter!) Docs and light touch refactorings for readability * Skip all failing tests * Document the single-test-skipping madness * Update tests to latest commit on the `develop` branch * Rename test skipping types to reflect actual purpose * Switch to skipping individual tests in currents.json Add some logging to help debug skipping * Fix rpc test by curve fitting to new json test source file * Add refs to all issues for fixing failing&skipped json tests * Sort out the need for Clone for tests * [json-tests] populate state from genesis pod state (#11083) * [json-tests] populate state from genesis pod state * [json-tests] #11075 is resolved as well * [json-tests] #11076 hopefully too * [json-tests] #11077 🎉 * [json-tests] fix trailing comma * Update ethcore/src/json_tests/chain.rs Co-Authored-By: Andronik Ordian <write@reusable.software> * Add issue numbers to TODOs * Apply @ordians fix for wrong state_root * Warn on invalid RLP * Remove the `ci-skip-tests` feature

niklasad1 added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Sep 6, 2019

niklasad1 commented Sep 6, 2019

View reviewed changes

Comment thread ethcore/Cargo.toml Outdated

niklasad1 changed the title ~~[WIP]: cleanup json crate~~ cleanup json crate Sep 6, 2019

niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M2-config 📂 Chain specifications and node configurations. M4-core ⛓ Core client code / Rust. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Sep 6, 2019

niklasad1 added this to the 2.7 milestone Sep 6, 2019

niklasad1 marked this pull request as ready for review September 6, 2019 19:49

dvdplm approved these changes Sep 8, 2019

View reviewed changes

debris approved these changes Sep 9, 2019

View reviewed changes

niklasad1 and others added 9 commits September 10, 2019 08:54

[json]: cleanup

be9d1fb

write something here....

nit: commit new/moved files

aa6a7b8

nit: remove needless features

b9cd7a6

nits

b18a105

fix(grumbles): use explicit import DifficultyTest

cbfcd8e

fix(grumbles): remove needless type hints

797da9a

fix(grumble): docs from -> used by

e91bca9

Co-Authored-By: David <dvdplm@gmail.com>

fix(grumbles): use explicit imports

6877c6a

fix(grumble): merge tx and tx_with_signing_info

7bbd397

niklasad1 force-pushed the na-cleanup-json-crate branch from e467ea0 to a51657d Compare September 10, 2019 09:42

fix(grumbles): resolve introduced TODO's

4837348

niklasad1 force-pushed the na-cleanup-json-crate branch from a51657d to 4837348 Compare September 10, 2019 09:49

dvdplm merged commit d311beb into master Sep 10, 2019

dvdplm deleted the na-cleanup-json-crate branch September 10, 2019 18:46

dvdplm reviewed Sep 10, 2019

View reviewed changes

niklasad1 mentioned this pull request Sep 11, 2019

[spec] add istanbul test spec #11033

Merged

s3krit mentioned this pull request Sep 25, 2019

v2.6.4-beta #11090

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup json crate#11027

cleanup json crate#11027
dvdplm merged 10 commits into
masterfrom
na-cleanup-json-crate

niklasad1 commented Sep 6, 2019 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvdplm Sep 8, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debris left a comment

Uh oh!

dvdplm Sep 10, 2019

Uh oh!

dvdplm Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

niklasad1 commented Sep 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvdplm Sep 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debris left a comment

Choose a reason for hiding this comment

Uh oh!

dvdplm Sep 10, 2019

Choose a reason for hiding this comment

Uh oh!

dvdplm Sep 10, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

niklasad1 commented Sep 6, 2019 •

edited

Loading