Skip to content
This repository was archived by the owner on Oct 14, 2020. It is now read-only.

Conversation

@jhoffner
Copy link
Member

@jhoffner jhoffner commented Aug 30, 2017

This adds a number of libraries to the node.docker image to work with Ethereum. This is different than the solidity.docker file, which is optimized for working specifically with testing large custom contracts. For this case, we are working with either inline compiled or example contracts and more focused on using the Ethereum API's.

Reservations

These API's seem to have stabilized, but I'm concerned that they may still be in flux still and we will have version issues if too much content is built on top of these. I found a number of outdated examples online. With that said, web3 is now at v1.0 (beta) and is the main way of interacting with the API.

Riders

  • removed some fringe node modules that are not known to have ever been used, in order to trim things down
  • added enzyme modules (adding test for these would be ideal, @Dan-Nolan maybe you can work on that when you update the react packages?)

@jhoffner jhoffner requested review from Dan-Nolan and kazk August 30, 2017 06:01
RUN mocha -t 10000 test/runners/typescript_spec.js
#RUN mocha -t 10000 --recursive test/runners/javascript/
#RUN mocha -t 10000 test/runners/coffeescript_spec.js
#RUN mocha -t 10000 test/runners/typescript_spec.js
Copy link
Member

Choose a reason for hiding this comment

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

Why are the tests commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, I meant to add those back in. I knew I was going to forget that.


const runner = require('../../../runner');

describe('ethereum', function() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm glad I had split javascript_spec.js. Tests are much easier to find and write :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah great idea! One thing I did notice is that the new files no longer have _spec as a suffix, and the naming scheme switched to kebob case. Was that on purpose?

Copy link
Member

Choose a reason for hiding this comment

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

_spec seems redundant to me because these are under test/, but if you want them back I don't mind. kebab case file names are more common in JS projects and matches rest of the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I can't keep them straight. I could have swore this repo was underscore cased, but apparently its just the specs.

The useful thing about _specs is that when using file find its nice to see that a file is a spec (in case there are other files named the same), but also because there may be helper files within the directory that are not actually specs. Not a big deal though.

Copy link
Contributor

@Dan-Nolan Dan-Nolan left a comment

Choose a reason for hiding this comment

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

Cool! Looks like a good way to do some more general understanding-the-blockchain challenges. Custom implementations of miners and the merkle-patricia-tree would be interesting as well.

Sure I'll look into adding some enzyme flavored tests. It was a considered library when I first wrote the react challenges, but I stayed away anything non-standard at the time. Jest is another one that might also be requested by teams testing React, but that would replace mocha.

@jhoffner
Copy link
Member Author

What the hell, Travis stopped caring about this PR 😟

@jhoffner
Copy link
Member Author

@Dan-Nolan does Enzyme really give users anything they didn't have already? I'm now debating if we should actually accommodate their request.

@kazk
Copy link
Member

kazk commented Aug 30, 2017

Apparently angularjs.org is not responding :(

$ curl -I http://angularjs.org
curl: (7) Failed to connect to angularjs.org port 80: Operation timed out

@OverZealous
Copy link
Contributor

Looks like they got it back up again (that was annoying this morning—no docs!)

@OverZealous
Copy link
Contributor

I restarted the build.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants