Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

JSON Test suite #11801

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

JSON Test suite #11801

wants to merge 2 commits into from

Conversation

adria0
Copy link

@adria0 adria0 commented Jun 26, 2020 •
edited
Loading

This PR removes the JSON tests embedded in the code and creates a JSON file with the instructions about how to execute those tests. This allows to:

  • Do not manually sync manually the tests especified in the code with the contents of the external git module
  • Having a JSON file specifying where the tests are, and specifying which of them are executed and which ones not
  • Eventually, allow to execute it using the evmbin tool. This will be done in another PR.

The changes include:

  • ethcore/res/ethereum/runner/full.json is the test suite specification file, the JSON structs are in json/src/test_helpers/ethspec.rs
  • Added an executor for the tests ethcore/src/json_tests/runner.rs
  • Modified the tests executors to create an uniform output
  • Removed embeeded tests, and changed for only one with an executor

@adria0 adria0 requested review from dvdplm and vorot93 June 26, 2020 18:25
@adria0 adria0 added the A0-pleasereview 🤓 Pull request needs code review. label Jun 26, 2020
@adria0 adria0 linked an issue Jun 26, 2020 that may be closed by this pull request
Automatic json test files discovery #11669
Open
sorpaas
sorpaas approved these changes Jul 29, 2020
View reviewed changes
Copy link
Collaborator

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@sorpaas sorpaas added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. M4-core ⛓ Core client code / Rust. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 29, 2020
@sorpaas
Copy link
Collaborator

sorpaas commented Jul 29, 2020

The test failure is extremely weird:

 error: failed to download from `https://crates.io/api/v1/crates/natpmp/0.2.0/download`

@sorpaas sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Jul 31, 2020
@sorpaas
Copy link
Collaborator

sorpaas commented Jul 31, 2020

(Looks like test failure has been fixed.)

This was referenced Aug 7, 2020
Closed
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sorpaas sorpaas sorpaas approved these changes

@dvdplm dvdplm Awaiting requested review from dvdplm

@vorot93 vorot93 Awaiting requested review from vorot93

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automatic json test files discovery
2 participants
@adria0 @sorpaas