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

Run tests without subprocess and split tests and examples #1105

Merged
merged 3 commits into from
Jan 22, 2020

Conversation

dapplion
Copy link
Contributor

🦅 Pull Request

Builds on #1099.

  • Instead of calling a node script via a subprocess, the example guide is run directly within node, which provides better debugging and clarity.
  • Test runs are divided into two NPM scripts npm run test and npm run test:integration. It can make sense to not run the examples test on the git hooks, and run them only on CI. We could also skip coverage for the guides / examples with a similar technique

🚨 Test instructions

npm run test:integration

@eternauta1337 eternauta1337 added the 🛠️ toolkit Toolkit for accessing Aragon utilities programmatically. label Jan 2, 2020
Comment on lines 27 to 38
const ensRegistryAddress = '0x5f6f7e8cc7346a11ca2def8f827b7a0b612c56a1'
const options = {
web3,
provider: web3.eth.currentProvider,
ipfs: {
rpc: { protocol: 'http', host: 'localhost', port: 5001, default: true },
gateway: 'http://localhost:8080/ipfs',
},
registryAddress: ensRegistryAddress,
ensRegistryAddress,
'ens-registry': ensRegistryAddress,
}
Copy link
Contributor

@0xGabi 0xGabi Jan 2, 2020

Choose a reason for hiding this comment

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

Suggested change
const ensRegistryAddress = '0x5f6f7e8cc7346a11ca2def8f827b7a0b612c56a1'
const options = {
web3,
provider: web3.eth.currentProvider,
ipfs: {
rpc: { protocol: 'http', host: 'localhost', port: 5001, default: true },
gateway: 'http://localhost:8080/ipfs',
},
registryAddress: ensRegistryAddress,
ensRegistryAddress,
'ens-registry': ensRegistryAddress,
}
const options = {
web3,
provider: web3.eth.currentProvider,
apm: {
ipfs: {
rpc: { protocol: 'http', host: 'localhost', port: 5001, default: true },
gateway: 'http://localhost:8080/ipfs',
},
ensRegistryAddress: '0x5f6f7e8cc7346a11ca2def8f827b7a0b612c56a1',
},
}

app: votingAddress,
method: 'newVote',
params: [script, 'Execute multiple votes'],
apm: options,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
apm: options,
apm: options.apm,

web3,
'membership-template.aragonpm.eth',
'latest',
options
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
options
options.apm

Copy link
Contributor

@0xGabi 0xGabi left a comment

Choose a reason for hiding this comment

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

The option object was a bit awkward. Also in the example only the apm configuration was utilized. Do we want to remove the provider and web3? 🤔

@dapplion
Copy link
Contributor Author

dapplion commented Jan 3, 2020

The option object was a bit awkward. Also in the example only the apm configuration was utilized. Do we want to remove the provider and web3?

@0xGabi I agree, the option object is awful but that's what's required right now to make it work. This should be used a hint to improve the interface / API of the toolkit.

@0xGabi 0xGabi added this to the Sprint 1 milestone Jan 16, 2020
@0xGabi 0xGabi changed the base branch from develop to move-logic2 January 22, 2020 15:23
@0xGabi 0xGabi merged commit bd781fd into aragon:move-logic2 Jan 22, 2020
@dapplion dapplion deleted the examples-organization branch January 23, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ toolkit Toolkit for accessing Aragon utilities programmatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants