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 API tests for index, stateManager #364

Merged
merged 1 commit into from Oct 2, 2018

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Sep 28, 2018

This PR is part of the series for adding API test coverage. It:

  • Adds tests for index.js
  • Adds basic tests for stateManager.js

Signed-off-by: Sina Mahmoodi <itz.s1na@gmail.com>
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Hi Sina, extremely imaginative with all the cases you came up with, great!

Everything looks good, will approve and merge. Was this the last of the PR series or is there still something missing?

@@ -2,6 +2,7 @@ const Buffer = require('safe-buffer').Buffer
const utils = require('ethereumjs-util')

module.exports = {
fake: true,
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

@@ -227,7 +227,7 @@ proto.revert = function (cb) {
//
proto.getStateRoot = function (cb) {
var self = this
self.cacheFlush(function (err) {
self.cache.flush(function (err) {
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

st.equal(vm.stateManager.trie.root, util.KECCAK256_RLP, 'it has default trie')
st.ok(vm.blockchain.fake, 'it has fake blockchain by default')
st.end()
})
Copy link
Member

Choose a reason for hiding this comment

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

Ok. This is so cool, that this stuff get's explicit here. 👍

t.test('should be able to activate precompiles', (st) => {
let vm = new VM({ activatePrecompiles: true })
st.notEqual(vm.stateManager.trie.root, util.KECCAK256_RLP, 'it has different root')
st.end()
Copy link
Member

Choose a reason for hiding this comment

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

Nice check, ok.

vm = new VM({ chain: 'mainchain', hardfork: 'homestead' })
st.fail('should have failed for invalid chain')
} catch (e) {
st.ok(e.message.includes('not supported'))
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

const vm = setupVM()
await runBlockchainP(vm)
st.end()
})
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

.then(() => st.fail('it hasn\'t returned any errors'))
.catch((e) => {
st.equal(e.message, 'test', 'it has correctly propagated runBlock\'s error')
st.end()
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

await runBlockchainP(vm)

st.end()
})
Copy link
Member

Choose a reason for hiding this comment

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

Ok.


st.equal(stateManager._common.hardfork(), 'byzantium', 'it has default hardfork')
st.end()
})
Copy link
Member

Choose a reason for hiding this comment

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

Ok.


st.equal(res.balance.toString('hex'), 'fff384')
st.end()
})
Copy link
Member

Choose a reason for hiding this comment

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

Ok.

@holgerd77 holgerd77 merged commit 73b089d into ethereumjs:master Oct 2, 2018
@s1na
Copy link
Contributor Author

s1na commented Oct 2, 2018

Hey @holgerd77, thanks, I'm glad the quality was sufficient :)
Yes, this was the last PR I had in mind. If there's something missing, please let me know.

@s1na s1na deleted the test/api-index branch October 2, 2018 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants