runCode.js: add pc to opt args | initial API tests
#406
Conversation
|
I would be feeling more comfortable if we would have some API tests for this. This would in the first place need some basic setup for tests on |
|
@holgerd77 👍 I'm going to add some tests next week |
|
Great! |
As a user, I wan't to pass the initial program counter to the `runCode` method to gain the ability to start the execution at a specific step.
|
@holgerd77 Ready for review 👍 |
tests/tester.js
Outdated
| @@ -249,6 +249,7 @@ function runAll () { | |||
| require('./tester.js') | |||
| require('./genesishashes.js') | |||
| require('./constantinopleSstoreTest.js') | |||
| require('./programCounter.js') | |||
There was a problem hiding this comment.
Sorry, this is really completely our fault and I have directly created a test cleanup PR #437 on this to avoid future confusion:
The runAll() method is not used for quite some time and is also not integrated into CI, so currently your tests are not run.
Can you just add a file to the tests/api folder instead? Then the tests are run along the npm run testAPI command.
Also: the naming with programCounter.js is far too specific. Can we change this to some to-be-expanded entry point to generally test runCode functionality and can you therefore change the filename to runCode.js?
holgerd77
changed the title
pc to the runCode optional argspc to runCode opt args | initial API tests
holgerd77
changed the title
pc to runCode opt args | initial API testspc to opt args | initial API tests
| @@ -108,6 +109,12 @@ module.exports = function (opts, cb) { | |||
|
|
|||
| // iterate through the given ops until something breaks or we hit STOP | |||
| function runVm (err) { | |||
| // Check that the programCounter is in range. Does not overwrite the previous err, if any. | |||
| const pc = runState.programCounter | |||
| if (!err && pc !== 0 && (pc < 0 || pc >= runState.code.length)) { | |||
| }) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Love this new test setup so so much 👍 😄 ✨, this makes the whole code execution so much more approachable and serves as some really valuable usage reference!
Also super transparent how to add on this, great basis for further tests, thanks a lot for the thorough work here!
|
Do you need some release on this in any way? |
|
@holgerd77 Not 'yet' 😄 . I start working on an interface in the next days to partly/fully solve #410, so I'm going to stick with my fork until that is solved 👍 |
|
😄 |
As a user, I wan't to pass the initial program counter
to the
runCodemethod to gain the ability to start theexecution at a specific step.