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

Add manifest formatting rules #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,21 @@ def validate_v3(schema):
def _validate(*args, **kwargs):
validate(args[0], schema, cls=validator_for(schema, Draft7Validator))
return _validate

@pytest.fixture
def validate_manifest_formatting():
def _validate(*args, **kwargs):
raw_manifest = args[0]
try:
manifest_dict = json.loads(raw_manifest)
except json.JSONDecodeError:
raise Exception("Invalid JSON")

if raw_manifest[-1:] == "\n":
raise Exception("Invalid trailing newline")

sorted_manifest = json.dumps(manifest_dict, sort_keys=True, separators=(",", ":"))
if raw_manifest != sorted_manifest:
raise Exception("Invalid manifest has unsorted keys, duplicate keys, or is not tightly packed.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems non-trivial to get more precise testing for these different cases, so I just lumped them together for now. But I can look into enhancing this test to explicitly cover each test case if it seems worthwhile. The "reason" in each fixture is more precise in what the particular cause of error is.


return _validate
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/duplicateKeys0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\":\"ethpm/3\",\"manifest\":\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "duplicate keys"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/duplicateKeys1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"contractTypes\":{\"ABCContract\":{\"abi\":[]},\"ABCContract\":{\"abi\":[]}},\"manifest\":\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "duplicate keys"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/invalidJson0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{{\"manifest\":\"ethpm/3\"}}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid JSON"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/invalidJson1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\",\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid JSON"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/trailingNewline0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\":\"ethpm/3\"}\n",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid trailing newline"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/trailingNewline1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "\n{\"manifest\":\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "is not tightly packed"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/trailingNewline2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\":\"ethpm/3\"}\n\n",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid trailing newline"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/unsortedKeys0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"contractTypes\":{\"XYZContract\":{},\"ABCContract\":{}},\"manifest\":\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid manifest has unsorted keys"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/unsortedKeys1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\":\"ethpm/3\",\"contractTypes\":{\"MyContract\":{}}}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid manifest has unsorted keys"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/unsortedKeys2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"manifest\":\"ethpm/3\",\"contractTypes\":{\"ABCContract\":{\"userdoc\":{},\"abi\":[]}}}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "Invalid manifest has unsorted keys"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/whitespace0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\"contractTypes\": {\"ABCContract\":{}},\"manifest\":\"ethpm/3\"}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "is not tightly packed"
}
}
9 changes: 9 additions & 0 deletions tests/fixtures/formatting/invalid/whitespace1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"package": "{\n \"contractTypes\":{\n \"ABCContract\":{}\n },\n \"manifest\":\"ethpm/3\"\n}",
"testCase": "invalid",
"errorInfo": {
"errorCode": "N0010",
"errorPointer": "/",
"reason": "is not tightly packed"
}
}
2 changes: 1 addition & 1 deletion tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
'wallet'
)
)
def test_schema_accepts_v3_examples(validate_v3, ethpm_spec_dir, example):
def test_schema_validates_v3_examples(validate_v3, ethpm_spec_dir, example):
examples_dir = ethpm_spec_dir / 'examples'
manifest = json.loads((examples_dir / example / 'v3.json').read_text())
assert validate_v3(manifest) is None
31 changes: 21 additions & 10 deletions tests/test_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,39 @@
)

FIXTURE_DIR = Path(__file__).parent.parent / 'tests' / 'fixtures'
SCHEMA_VALIDATION_DIR = FIXTURE_DIR / 'schemaValidation'
FORMATTING_DIR = FIXTURE_DIR / 'formatting'


def pytest_generate_tests(metafunc):
if 'valid_fixture' in metafunc.fixturenames:
valid_fixtures = [fixture for fixture in FIXTURE_DIR.glob('**/*.json') if fixture.parent.name == 'valid']
metafunc.parametrize("valid_fixture", valid_fixtures)
elif 'invalid_fixture' in metafunc.fixturenames:
invalid_fixtures = [fixture for fixture in FIXTURE_DIR.glob('**/*.json') if fixture.parent.name == 'invalid']
metafunc.parametrize("invalid_fixture", invalid_fixtures)
if 'valid_schema_fixture' in metafunc.fixturenames:
valid_fixtures = [fixture for fixture in SCHEMA_VALIDATION_DIR.glob('**/*.json') if fixture.parent.name == 'valid']
metafunc.parametrize("valid_schema_fixture", valid_fixtures)
elif 'invalid_schema_fixture' in metafunc.fixturenames:
invalid_fixtures = [fixture for fixture in SCHEMA_VALIDATION_DIR.glob('**/*.json') if fixture.parent.name == 'invalid']
metafunc.parametrize("invalid_schema_fixture", invalid_fixtures)
elif 'invalid_formatting_fixture' in metafunc.fixturenames:
invalid_fixtures = [fixture for fixture in FORMATTING_DIR.glob('**/*.json') if fixture.parent.name == 'invalid']
metafunc.parametrize("invalid_formatting_fixture", invalid_fixtures)


def test_valid_fixtures(valid_fixture, validate_v3):
fixture_json = json.loads(valid_fixture.read_text())
def test_valid_schema_fixtures(valid_schema_fixture, validate_v3):
fixture_json = json.loads(valid_schema_fixture.read_text())
manifest_json = json.loads(fixture_json['package'])
assert fixture_json['testCase'] == 'valid'
assert validate_v3(manifest_json) is None


def test_invalid_fixtures(invalid_fixture, validate_v3):
fixture_json = json.loads(invalid_fixture.read_text())
def test_invalid_schema_fixtures(invalid_schema_fixture, validate_v3):
fixture_json = json.loads(invalid_schema_fixture.read_text())
manifest_json = json.loads(fixture_json['package'])
escaped_reason = re.escape(fixture_json['errorInfo']['reason'])
assert fixture_json['testCase'] == 'invalid'
with pytest.raises(jsonValidationError, match=escaped_reason):
validate_v3(manifest_json)


def test_invalid_formatting_fixtures(invalid_formatting_fixture, validate_manifest_formatting):
fixture_json = json.loads(invalid_formatting_fixture.read_text())
with pytest.raises(Exception, match=fixture_json['errorInfo']['reason']):
validate_manifest_formatting(fixture_json['package'])