Create diff tests for minor hardforks #635
Conversation
Codecov ReportBase: 68.75% // Head: 68.76% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #635 +/- ##
=======================================
Coverage 68.75% 68.76%
=======================================
Files 481 481
Lines 26836 26836
=======================================
+ Hits 18451 18453 +2
+ Misses 8385 8383 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
I wonder if these checks would be better suited to ethereum-spec-lint? I believe the Lint base class already gives you the previous hard fork, and the walk_sources function will be useful.
Could filter for forks ending with _glacier and make sure the only changed node is the fork block. Wouldn't work for the DAO fork, but maybe that could be its own special lint.
| code = re.sub(re.compile('""".*?"""', re.DOTALL), "", code) | ||
| code = re.sub(re.compile("#.*?\n"), "\n", code) | ||
| code = re.sub(re.compile("\n\s*\n"), "\n", code) |
There was a problem hiding this comment.
Haven't tested, but I think you can replace the compile with just a plain string:
| code = re.sub(re.compile('""".*?"""', re.DOTALL), "", code) | |
| code = re.sub(re.compile("#.*?\n"), "\n", code) | |
| code = re.sub(re.compile("\n\s*\n"), "\n", code) | |
| code = re.sub('""".*?"""', "", code, flags=re.DOTALL) | |
| code = re.sub("#.*?\n", "\n", code) | |
| code = re.sub("\n\s*\n", "\n", code) |
| process_source_code(old_source_path, old_processed_path) | ||
| process_source_code(new_source_path, new_processed_path) | ||
|
|
||
| diffs = subprocess.run( | ||
| ["diff", old_processed_path, new_processed_path], | ||
| capture_output=True, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
Hm... do you think it would be possible to implement a solution using Python's ast? You could walk the tree and remove docstrings and comments, plus whitespace would already be taken care of. Wouldn't need to shell out to diff that way either, or write files.
Apparently there's no built-in comparison function but there's a post with some ideas on how to do it.
I guess the hardest part of this approach is specifying what changes are allowed. Could just hard code the test for *_glacier forks for now I guess.
There was a problem hiding this comment.
Sure. I will look into it.
|
Closing this in favour of the linting approach #639 |
(closes #605 )
What was wrong?
The minor hard forks do not currently have any tests.
Related to Issue #605
How was it fixed?
Create diff tests control for diffs in the hard fork source code, with the previous fork as the reference.
Cute Animal Picture