-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
WIP: e2e tests #2688
WIP: e2e tests #2688
Conversation
Originally based this off of the v1.x branch, working on rebasing it off of 1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! I think web3.js desperately needs this.
Added a few minor comments.
I'm happy to contribute with a few specific tests but we should sync effort this time. :) I was just about to push a similar PR.
@@ -0,0 +1,3 @@ | |||
FROM node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ganache is publishing docker images too: trufflesuite/ganache-cli
const url = protocol + '://' + host + ':' + rpcPort; | ||
return describe('Web3 integration tests for: ' + url, () => { | ||
before(function(done) { | ||
this.timeout(30000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it waiting for the provider to be ready after container launch?
Wouldn't be better to just wait for the port to be open before start any tests? Here it may still assert if it's connected (e.g. web3.eth.net.isListening()
)
I've closed this PR because of PR #2693 |
@@ -19,6 +19,8 @@ | |||
"docs": "cd docs; make html;", | |||
"lint": "jshint *.js packages", | |||
"test": "mocha; jshint *.js packages", | |||
"e2e": "docker-compose down && docker-compose build && docker-compose run --entrypoint 'npm run e2e:mocha' test_runner", | |||
"e2e:mocha": "mocha --recursive ./e2e/**/*.spec.js -R spec --exit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure of introducing a new testing framework. What's the reason?
I was struggling with the Jest magic in the project but adding yet an other framework might increase package complexity even more.
Description
Starting the scaffolding of the e2e tests
Start of the work for #2682
Type of change
Checklist:
npm run test
in the root folder with success and extended the tests if necessary.npm run build
in the root folder and tested it in the browser and with node.npm run dtslint
in the root folder and tested that all my types are correct